On Mon, 2021-03-15 at 15:53 +0100, David Sterba wrote:
> On Sun, Mar 14, 2021 at 07:49:13PM +0100, Pierre Labastie wrote:
> > After sending the first version of the patch, I realized that it
> > was flawed, because of some formatting by the MUA. It took me
> > some time to set up an MTA so that git send-email works. Now the
> > patch should apply cleanly. Please remove the present paragraph by using
> > git am -c. Apologies for the inconvenience(s).
> > -- >8 --
> > Commit b3df561fbf has introduced the ability to convert extended
> > inode time precision on ext4, but this breaks builds on older distros,
> > where ext4 does not have the nsec time precision.
> > 
> > Commit c615287cc tried to fix that by testing the availability of
> > the EXT4_EPOCH_MASK macro, but the test is not complete.
> > 
> > This patch aims at fixing the macro test, and changes the
> > name of the associated HAVE_ macro, since the logic is reverted.
> > 
> > This fixes #353 when ext4 has nsec time precision. Note that
> > the test fails when ext4 does not have the nsec time precision.
> > Maybe the test shouldn't be run in that case?
> > 
> > Issue: #353
> > Signed-off-by: Pierre Labastie <pierre.labas...@neuf.fr>
> > ---
> >  configure.ac          | 8 ++++----
> >  convert/source-ext2.c | 6 +++---
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 612a3f87..dd6a5de7 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -251,10 +251,10 @@ else
> >  AC_DEFINE([HAVE_OWN_FIEMAP_EXTENT_SHARED_DEFINE], [0], [We did not define
> > FIEMAP_EXTENT_SHARED])
> >  fi
> >  
> > -HAVE_OWN_EXT4_EPOCH_MASK_DEFINE=0
> > -AX_CHECK_DEFINE([ext2fs/ext2_fs.h], [EXT4_EPOCH_MASK], [],
> > -               [HAVE_OWN_EXT4_EPOCH_MASK_DEFINE=1
> > -                AC_MSG_WARN([no definition of EXT4_EPOCH_MASK found,
> > probably old e2fsprogs, will use own definition, no 64bit time precision of
> > converted images])])
> > +AX_CHECK_DEFINE([ext2fs/ext2_fs.h], [EXT4_EPOCH_MASK],
> > +                [AC_DEFINE([HAVE_EXT4_EPOCH_MASK_DEFINE], [1],
> > +                   [Define to 1 if e2fsprogs defines EXT4_EPOCH_MASK])],
> > +               [AC_MSG_WARN([no definition of EXT4_EPOCH_MASK found,
> > probably old e2fsprogs, will use own definition, no 64bit time precision of
> > converted images])])
> 
> Inlining the AC_DEFINE to the check will skip defining the macro in case
> the EXT4_EPOCH_MASK does not exist and then the C #if won't work.
> 
>         HAVE_EXT4_EPOCH_MASK_DEFINE=0
>         AX_CHECK_DEFINE(...
>                 HAVE_EXT4_EPOCH_MASK_DEFINE=1,...)
> 
>         if x"$HAVE_EXT4_EPOCH_MASK_DEFINE"; then
>                 AC_DEFINE([HAVE_EXT4_EPOCH_MASK_DEFINE], [1])
>         else
>                 AC_DEFINE([HAVE_EXT4_EPOCH_MASK_DEFINE], [0])
>         fi

Since autoheader is used, and autoheader uses AC_DEFINE macros to generate
config.h.in, there is a risk that having two AC_DEFINE macros for the same 
identifier generates a conflict. I've not been able to find what it does in
that case in the autoconf doc.

OTOH, an undefined identifier is replaced by a zero when expanding a #if in the
C preprocessor (this is in all the norms I have access to: C89 and C99), that
is:

#if UNDEFINED_IDENTIFIER
is equivalent to
#if 0

Except if a compiler does not respect the norm, what I have proposed works (I
have only tested with gcc, and it does what it is supposed to do).

> 
> This should work, maybe it's not the shortest way to write that but I
> can't find anything better.

Now, I've tested your version, and it works too. So, up to you...

Pierre

Reply via email to