On 17 Oct 2022 15:16, Indu Bhagat via Binutils wrote: > --- /dev/null > +++ b/libsframe/Makefile.am > > +SUBDIRS = testsuite since this is new code, can you avoid recurisve makes ? rename the subdir Makefile.am to local.mk and include them in this Makefile.am instead. > +BASEDIR = $(srcdir)/.. you don't actually use this anywhere > +AM_CPPFLAGS = -D_GNU_SOURCE -I$(srcdir) -I$(srcdir)/../include -I$(srcdir)/../libctf you called AC_USE_SYSTEM_EXTENSIONS, so why do you need to use -D_GNU_SOURCE ? > +AM_CFLAGS = -std=gnu99 @ac_libsframe_warn_cflags@ @warn@ @c_warn@ @WARN_PEDANTIC@ @WERROR@ most other projects have already moved to C11. why do you have to use such an ancient version ? > +libsframe_la_CPPFLAGS = -D_GNU_SOURCE -I$(srcdir) -I$(srcdir)/../include \ > + -I$(srcdir)/../libctf you should be using $(AM_CPPFLAGS) instead of manually duplicating the value > --- /dev/null > +++ b/libsframe/configure.ac > > +AC_CONFIG_MACRO_DIR(..) > +AC_CONFIG_MACRO_DIR(../config) > +AC_CONFIG_MACRO_DIR(../bfd) you're setting ACLOCAL_AMFLAGS already. pretty sure you don't need these. > +MISSING=`cd $ac_aux_dir && ${PWDCMD-pwd}`/missing > +AC_CHECK_PROGS([ACLOCAL], [aclocal], [$MISSING aclocal]) > +AC_CHECK_PROGS([AUTOCONF], [autoconf], [$MISSING autoconf]) > +AC_CHECK_PROGS([AUTOHEADER], [autoheader], [$MISSING autoheader]) why do you need this logic ? isn't this covered by AM_MAINTAINER_MODE ? > +# Figure out what compiler warnings we can enable. > +# See config/warnings.m4 for details. use `dnl` for comments, not `#` > +ACX_PROG_CC_WARNING_OPTS([-W -Wall -Wno-narrowing -Wwrite-strings \ > + -Wmissing-format-attribute], [warn]) > +ACX_PROG_CC_WARNING_OPTS([-Wstrict-prototypes -Wmissing-prototypes \ > + -Wold-style-definition], [c_warn]) > +ACX_PROG_CC_WARNING_ALMOST_PEDANTIC([-Wno-long-long]) > + > +# Only enable with --enable-werror-always until existing warnings are > +# corrected. > +ACX_PROG_CC_WARNINGS_ARE_ERRORS([manual]) this project is new. why aren't the warnings fixed to begin with ? > +DEJAGNU_CHECK_VERSION > +AM_CONDITIONAL([HAVE_COMPAT_DEJAGNU], [test "x$ac_cv_dejagnu_compat" = "xyes"]) > + > +COMPAT_DEJAGNU=$ac_cv_dejagnu_compat > +AC_SUBST(COMPAT_DEJAGNU) > + > +AM_MAINTAINER_MODE > +AM_INSTALL_LIBBFD > +ACX_PROG_CC_WARNING_OPTS([-Wall], [ac_libsframe_warn_cflags]) you already probed -Wall above. do you really need to do it again ? > +AC_FUNC_MMAP > +AC_CHECK_HEADERS(byteswap.h endian.h) > + > +dnl Check for bswap_{16,32,64} > +AC_CHECK_DECLS([bswap_16, bswap_32, bswap_64], [], [], [[#include ]]) > +AC_CHECK_DECLS([asprintf, vasprintf, stpcpy]) you aren't using most of this. are you just copying & pasting from another tree ? > +GNU_MAKE_JOBSERVER do you actually need this ? > --- /dev/null > +++ b/libsframe/testsuite/libsframe.decode/Makefile.am > @@ -0,0 +1,17 @@ > +if HAVE_COMPAT_DEJAGNU > + check_PROGRAMS = be-flipping frecnt-1 frecnt-2 > +else > + check_PROGRAMS = > +endif this style complicates things. use: check_PROGRAMS = if HAVE_COMPAT_DEJAGNU check_PROGRAMS += ... endif -mike