Subject: Re: scan_ffs from OpenBSD ported and improved.
To: Juan RP <email@example.com>
From: Julio M. Merino Vidal <firstname.lastname@example.org>
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" <email@example.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
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
Julio M. Merino Vidal <firstname.lastname@example.org>
The NetBSD Project - http://www.NetBSD.org/