Subject: Re: scan_ffs from OpenBSD ported and improved.
To: Julio M. Merino Vidal <jmmv84@gmail.com>
From: Juan RP <juan@xtrarom.org>
List: tech-userlevel
Date: 06/06/2005 14:12:30
On Mon, 06 Jun 2005 14:06:48 +0000
"Julio M. Merino Vidal" <jmmv84@gmail.com> wrote:
> On Mon, 2005-06-06 at 13:44 +0200, Juan RP wrote:
> > Hi, finally I've fixed all problems I had reported in my previous email
> > "Porting scan_ffs from OpenBSD", the following is the example of
> > this utility to find FFSv2 partitions with different blocksizes:
> [...]
> > Please test and review, I'd want to commit this on the next days if there
> > isn't any objection.
>
> >From a very quick review, I see:
> - Auxiliary functions are not static.
Should be declared as 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.
For operations like (blk+(n/512)*foo), I wanted to leave them without spaces.
> - Functions that return values which are discarded should be prefixed
> with (void). I.e., (void)printf("blah");
Ok!
> - Options in getopt should be sorted alphabetically.
Ok!
> - Most NetBSD utilities don't have '-h' for help, while this one does.
Should "-h" be removed then?
> - Where are the patches to modify the build infrastructure to build the
> utility?
I haven't added these patches, because they are trivial to add.
> - And the manual page?
I'll write the manual page tomorrow, I'd want to explain its current state
finding FFSv1 partitions.
Thanks for your commments.