[PATCH 12/14] xfsprogs: make fsr use mntinfo when there is no mntent

Jan Tulak jtulak at redhat.com
Thu Sep 24 09:38:49 CDT 2015


On Wed, Sep 23, 2015 at 5:36 AM, Dave Chinner <david at fromorbit.com> wrote:

> On Tue, Sep 15, 2015 at 11:59:22AM +0200, Jan Tulak wrote:
> > @@ -202,6 +205,27 @@ find_mountpoint(char *mtab, char *argname, struct
> stat64 *sb)
> >       }
> >
> >       while ((t = getmntent(mtabp))) {
> > +#elif defined(HAVE_GETMNTINFO)
> > +     struct statfs   *stats;
> > +     int error, i, count;
> > +     // because "t" is a pointer, but we don't need to use
> > +     // malloc for this usage
> > +     struct mntent t_tmp;
> > +     t = &t_tmp;
> > +
> > +
> > +     error = 0;
> > +     if ((count = getmntinfo(&stats, 0)) < 0) {
> > +             fprintf(stderr, _("%s: getmntinfo() failed: %s\n"),
> > +                             progname, strerror(errno));
> > +             return 0;
> > +     }
> > +
> > +     for (i = 0; i < count; i++) {
> > +             mntinfo2mntent(&stats[i], t);
> > +#else
> > +# error "How do I extract info about mounted filesystems on this
> platform?"
> > +#endif
>
> No, please don't do that. Having a loop iterator split across two
> separate defines is unmaintainable. Write two separate functions
> with the different loop iterators, then factor the common bit out
> of them into a single function.
>
>
I did a little refactoring to solve it. What I would like to ask ab​out is
this:
When I can put ifdef just inside of a function like fnc(void) { #ifdef...
#else ... #endif }, with little to no code outside of the ifdef, is it
better to put the ifdef outside, or keep it inside?

I suppose the outside placement allows for easier changes for one of the
branches when there would be a new function just for a specific branch. But
on the other side, it makes it easier to forget the second function in
another branch when making some change later.

​Cheers,
Jan​

--
​-​

Jan Tulak
jtulak at redhat.com / jan at tulak.me
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://oss.sgi.com/pipermail/xfs/attachments/20150924/384d5b04/attachment.html>


More information about the xfs mailing list