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

[Jan Tulak <jtulak@xxxxxxxxxx>]

On Wed, Sep 23, 2015 at 5:36 AM, Dave Chinner <david@xxxxxxxxxxxxx> 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@xxxxxxxxxxÂ/ jan@xxxxxxxx