Re: Some nilfs comments

[Chris Mason <chris.mason-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]

On Wed, 2009-01-07 at 14:20 +0900, Ryusuke Konishi wrote:
> On Tue, 06 Jan 2009 14:55:09 -0500, Chris Mason wrote:
> > On Mon, 2008-12-22 at 18:07 +0900, Ryusuke Konishi wrote:
> > > > I haven't dug too deeply in yet, but if there are parts you're most
> > > > interested in comments on, please let me know.
> > > 
> > > Well, I feel that the following two matters are particularlly
> > > questionable and need to be checked:
> > > 
> > 
> > > - ioctl:
> > >   Ioctl interface (routines and structures) were implemented in an
> > >   own way.  These seems to be checked whether to comply with the rules
> > >   of ioctl design.
> > > 
> > 
> > It took me far too long to look at this, I'm sorry.  But, looking
> > through the ioctl code I think you'll have a few (easy to fix) problems.
> > 
> > struct nilfs_argv {
> >         void *v_base;
> >         size_t v_nmembs;        /* number of members */
> >         size_t v_size;          /* size of members */
> >         int v_index;
> >         int v_flags;
> > };
> > 
> > The structs that get passed from userland to kernel should use fixed
> > sized types.
> > 
> > I get around this by using u64s (and __u64s) everywhere.  It looks like
> > you have compat ioctls to fix this too, I find it easier to use the
> > fixed types.
> > 
> > Otherwise the ioctl code looks pretty reasonable.
> > 
>
> I think it actually should be fixed to comply with the kernel rules.
> The same problem is found in a few other ioctl structures.
> 
> I'd like to modify them and remove compat ioctls some time soon.

Strictly speaking, your code is correct.  I suspect that if you ask 10
people you'll get 6 different answers on the best way.  The u64 is my
personal preference ;)

-chris