Re: PATCH, RFC: 2.6 Documentation/Codingstyle

[viro]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/