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.