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

[Dave Chinner <david@fromorbit.com>]

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@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...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs