Subject: Re: scan_ffs from OpenBSD ported and improved.
To: Juan RP <juan@xtrarom.org>
From: Julio M. Merino Vidal <jmmv84@gmail.com>
List: tech-userlevel
Date: 06/07/2005 10:37:41
On Tue, 2005-06-07 at 01:22 +0200, Juan RP wrote:
> On Mon, 06 Jun 2005 14:06:48 +0000
> "Julio M. Merino Vidal" <jmmv84@gmail.com> wrote:
> 
> > >From a very quick review, I see:
> > - Auxiliary functions are not static.
> > - Inconsistent spacing: there are some commas with a space after them,
> >   while others don't have it; there are multiple operators without
> >   spaces around them while others have it; etc.
> >   See /usr/share/misc/style.
> > - Functions that return values which are discarded should be prefixed
> >   with (void).  I.e., (void)printf("blah");
> > - Options in getopt should be sorted alphabetically.
> > - Most NetBSD utilities don't have '-h' for help, while this one does.
> > - Where are the patches to modify the build infrastructure to build the
> >   utility?
> > - And the manual page?
> 
> Attached is the patch with all your suggestions fixed, except the second
> suggestion (inconsistent spacing in multiple operators), I leave them as is
> because I think they are better to understand IMHO (der mouse said so
> too).

You still have formatting problems WRT style.

There are several commas without spaces after them, and I'm sure that's
not on purpose.  Also, second-level indents (i.e., line continuations)
should be four spaces, not a tab.  See style again.

Local variable declarations should be sorted in a special way, and be
kept together.  This is also documented in style, and there is a
rationale about it too.  Please read that document.

I haven't checked the manual page extensively, but:
- The AUTHORS section should be improved; use the An and Aq macros.
- Operating system names are written with Ox and Nx, among others.
- Apply the "New sentence, new line" policy.
- Sort command line options alphabetically.

At last, you'd make the usage function clearer by using a single printf.
(At the moment, it looks like as if it were two lines, while it's just
one.)

Cheers

-- 
Julio M. Merino Vidal <jmmv84@gmail.com>
http://www.livejournal.com/users/jmmv/
The NetBSD Project - http://www.NetBSD.org/