Messages in this thread | | | Date | Tue, 27 Jan 2009 17:34:18 +0100 | From | Jan Kara <> | Subject | Re: Checkpatch false positive? |
| |
On Tue 27-01-09 18:06:01, Adrian Bunk wrote: > On Tue, Jan 27, 2009 at 04:49:05PM +0100, Jan Kara wrote: > > Hi, > > > > I've used checkpatch.pl to verify one of my patches. It complains: > > > > ERROR: trailing statements should be on next line > > #167: FILE: fs/quota/quota_tree.c:249: > > + for (i = 0, ddquot = buf + sizeof(struct qt_disk_dqdbheader); > > [...] > > i++, ddquot += info->dqi_entry_size); > > > > But the code looks like: > > for (i = 0, ddquot = buf + sizeof(struct qt_disk_dqdbheader); > > i < qtree_dqstr_in_blk(info) && !qtree_entry_unused(info, ddquot); > > i++, ddquot += info->dqi_entry_size); > > > > Which is IMHO correct. Maybe it's because the for has actually empty body > > and the ; is at the end of the line with for. But I didn't find anything in > > CodingStyle that would forbid > > for (...); > > and > > for (...) > > ; > > Looks a bit strange. > > for (...); is a common C programming error, usually it's some kind of: > > for(........); > do_something(); > > This code does something different than intended. > And yes, we had such bugs in the kernel. > > > for(........) > ; > > is correct. The "looks a bit strange" is what actually tells readers > what the code is doing (and that the author did it intentionally). OK, makes some sence. Thanks for explanation.
Honza -- Jan Kara <jack@suse.cz> SUSE Labs, CR
| |