Re: e2fsprogs patch for review (and other stuff)
Samuel and Guillem,
Thanks for your comments.
On Wed, 2011-10-05 at 02:00 +0200, Guillem Jover wrote:
> Hi!
>
> On Tue, 2011-10-04 at 23:30:14 +0200, Samuel Thibault wrote:
> > Svante Signell, le Tue 04 Oct 2011 17:53:49 +0200, a écrit :
> > > --- e2fsprogs-1.42~WIP-2011-10-01/lib/quota/mkquota.c 2011-09-18 20:06:52.000000000 +0200
> > > +++ e2fsprogs-1.42~WIP-2011-10-01.modified//lib/quota/mkquota.c 2011-10-04 15:25:26.000000000 +0200
> > > @@ -51,9 +51,13 @@
> > > char tmp[1024];
> > > qid_t id = (type == USRQUOTA) ? getuid() : getgid();
> > >
> > > +#ifndef __GNU__
> > > if (!quotactl(QCMD(Q_V2_GETQUOTA, type), fs->device_name, id, tmp))
> > > return 1;
> > > return 0;
> > > +#else
> >
> > Maybe rather use #ifdef Q_V2_GETQUOA?
>
> Right, I've noticed you tend to do this, but I think it's in general
> preferable to check for the positive condition: where is this
> supported, rather than the opposite.
>
> The function scope variables will not be needed either, so they can be
> ifdefed out too, or they might generate warnings.
They can not be #ifdef'd on Q_V2_GETQUOTA since they are defined in the
included header files. The only thing to check is quotactl defined in
quota.h. Adding a configure check for that file would enable testing if
quota.h exists and by that #ifdef out the call to quotatctl. Since quota
is not supported in Hurd, I'll disable the build of libquota in
debian/rules instead (except if there are other reasons for building it,
and getting rid of the PATH_MAX stuff?).
The other coding comments are appreciated, but since the code is part of
libquota I will not submit a patch on that part.
Reply to: