Messages in this thread | | | Date | Fri, 13 Feb 2004 16:18:34 +0000 | From | viro@parcelfa ... | Subject | Re: PATCH, RFC: 2.6 Documentation/Codingstyle |
| |
On Fri, Feb 13, 2004 at 02:55:13PM +0100, Giuliano Pochini wrote: > I propose to change "hard limit" to "soft limit" to avoid things like this: > > rc=idefloppy_begin_format(drive, inode, > file, > (int *)arg);
To avoid such things, we'd better a) note that idefloppy_begin_format() ignores its second and third arguments and thus shouldn't have them at all (done in 2.6) b) note that we are within if (...) { ... return -EBUSY; } else { ... our call ... } and thus can trim one indent level. (done in 2.6) c) note that we are within { idefloppy_floppy_t *floppy = drive->driver_data; ... our call ... } and compound operator is needed only to because of that declaration. At the same time, we already have idefloppy_floppy_t *floppy = drive->driver_data; in the beginning of function and neither drive nor drive->driver_data can change between those declarations. IOW, declaration in the compound statement can be dropped and statement itself - opened. (done in 2.6) d) note that it's static, so "idefloppy_" prefix is plain and simple idiocy. e) note that rc = begin_format(drive, (int *)arg); fits on the line just fine and is far more readable than crap above, with or without linewrap.
Next example, please?
While we are at it, if (drive->usage > 1) { /* Don't format if someone is using the disk */ several lines above is broken by design - fd = open("/dev/hdc", O_RDWR); if (fork() == 0) write(fd, big_buffer, sizeof(big_buffer); else ioctl(fd, IDEFLOPPY_IOCTL_FORMAT_START, &args); exit(0); will cheerfully pass that check and start format while the drive is very much in use. Driver (and hardware) will not be happy.
ioctl(2) - Just Say No... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |