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

Jan Tulak jtulak at redhat.com
Tue Sep 29 11:07:32 CDT 2015


On Fri, Sep 25, 2015 at 12:53 AM, Dave Chinner <david at fromorbit.com> wrote:

> On Thu, Sep 24, 2015 at 04:38:49PM +0200, Jan Tulak wrote:
> > 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?
>
> The idea is that the "little differences" are put in functions that
> end up in include/<platform>.h or libxfs/<platform>.c, so there are
> no ifdefs in any of the application or library code. The build will
> automatically include the correct function on the given platform,
> and so the application code does not need such ifdefs at all.
>
> e.g. you could implement these functions to abstract the differences
> away from xfs_fsr and any other code that iterates the mount table:
>
> struct mntent_cursor {
>         /* variables needed to track iteration of the mtab */
> };
>
> platform_first_mntent()
> platform_next_mntent()
> platform_end_mntent()
>
> and so the code would look like:
>
>         struct mntent_cursor    cursor;
>
>         mntent = platform_first_mntent(&cursor)
>
>         do {
>                 /* process mntent */
>         } while (mntent = platform_next_mntent(&cursor, mntent));
>
>         platform_end_mntent(&cursor);
>
> This completely abstracts the differences related to the the mount
> table traversal, and allows the aplication level code to be written
> in a clean, easily maintainable fashion...
>

​So something like what I just posted? I added the same code that works for
Linux into other platforms other than OS X. ​

​Because as it was there before,  it either works on them, ​or no one care.
And I really don't have an idea how to test it on Irix... :-)

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/20150929/75bf19e2/attachment.html>


More information about the xfs mailing list