Subject: Re: SCSI MMC device abstraction and UDF patch for review
To: None <tech-kern@netbsd.org>
From: Christos Zoulas <christos@astron.com>
List: tech-kern
Date: 12/21/2005 18:45:14
In article <Pine.LNX.4.61.0512211909130.18519@m24s24.vlinux.de>,
Hubert Feyrer  <feyrer@cs.stevens.edu> wrote:
>On Wed, 21 Dec 2005, Reinoud Zandijk wrote:
>> ftp://ftp.NetBSD.org/pub/NetBSD/misc/reinoud/src-diffs-scsi.20051222.gz
>
>There is no patch for cd.4 included.
>
>What can one do with those MMC ioctls?
>
>
>> ftp://ftp.NetBSD.org/pub/NetBSD/misc/reinoud/src-diffs-udf.20051222.gz
>
>mount_udf.8:
>  * No description of any of the flags (-m, -o, -t) is included
>  * filingsystem -> file system?
>  * Maybe add a HISTORY section to indicate when this hits NetBSD (4.0?)
>  * Maybe add a SEE ALSO section
>  * Maybe add a few more words to include all the nice buzzwords you
>    had in your mail (BlueRay, ...)?
>
>sys/fs/udf/udf_vfsops.c:
>  * indentation of those UDFD() calls looks strange

mount_udf:
- The -m flag should be split to -u uid, -g gid like the other mount calls.
- There should be no space after casts.
- You should use getprogname/setprogname
- You should use err/errx instead of fprintf and exit
- the old main function is not ansi.
- no point initializing the anonuid and anongid to -1 if you are going to exit
  if you cannot find nobody.nobody.
- uids and gids should be uid_t and gid_t not uint32_t.

General:
- no bcopy/bcmp/bzero; use their mem equivalents.
- the UDFD macro should be converted to be DPRINTF like as the rest of the
  kernel has it, and indented properly.
- no need to use extern before function declarations.
- you should always have spaces around operants i=0 or i+=2 is no good.
- you should never have spaces after opening or before closing parens eg.
  printf( "foo"); or printf("foo" ); is not following our style.
- continuation lines are indented by four spaces.
- There is no need to cast void * pointers to other types.
- Don't use // comments to temporary comment out code; either delete it
  or #ifdef notdef it.

These are many minor nits; in general it is fine!

christos