| Message ID | 20181029104650.24924-1-bluca@debian.org |
|---|---|
| State | Rejected, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | [iproute2] Use libbsd for strlcpy if available | expand |
On 10/29/18 4:46 AM, Luca Boccassi wrote: > If libc does not provide strlcpy check for libbsd with pkg-config to > avoid relying on inline version. > > Signed-off-by: Luca Boccassi <bluca@debian.org> > --- > This allows distro maintainers to be able to choose to reduce > duplication and let this code be maintained in one place, in the > external library. > > configure | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index 744d6282..1dd9ce84 100755 > --- a/configure > +++ b/configure > @@ -330,8 +330,16 @@ EOF > then > echo "no" > else > - echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG > - echo "yes" > + if ${PKG_CONFIG} libbsd --exists > + then > + echo 'CFLAGS += -include' `${PKG_CONFIG} libbsd --variable=includedir`'/bsd/string.h' \ > + `${PKG_CONFIG} libbsd --cflags` >>$CONFIG > + echo 'LDLIBS +=' `${PKG_CONFIG} libbsd --libs` >> $CONFIG > + echo "no" > + else > + echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG > + echo "yes" > + fi > fi > rm -f $TMPDIR/strtest.c $TMPDIR/strtest > } > How long has libbsd had an implementation of strlcpy? Would be safer to have a compile test to verify libbsd has it.
On Mon, 2018-10-29 at 09:27 -0600, David Ahern wrote: > On 10/29/18 4:46 AM, Luca Boccassi wrote: > > If libc does not provide strlcpy check for libbsd with pkg-config > > to > > avoid relying on inline version. > > > > Signed-off-by: Luca Boccassi <bluca@debian.org> > > --- > > This allows distro maintainers to be able to choose to reduce > > duplication and let this code be maintained in one place, in the > > external library. > > > > configure | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/configure b/configure > > index 744d6282..1dd9ce84 100755 > > --- a/configure > > +++ b/configure > > @@ -330,8 +330,16 @@ EOF > > then > > echo "no" > > else > > - echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG > > - echo "yes" > > + if ${PKG_CONFIG} libbsd --exists > > + then > > + echo 'CFLAGS += -include' `${PKG_CONFIG} libbsd -- > > variable=includedir`'/bsd/string.h' \ > > + `${PKG_CONFIG} libbsd --cflags` >>$CONFIG > > + echo 'LDLIBS +=' `${PKG_CONFIG} libbsd --libs` >> > > $CONFIG > > + echo "no" > > + else > > + echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG > > + echo "yes" > > + fi > > fi > > rm -f $TMPDIR/strtest.c $TMPDIR/strtest > > } > > > > How long has libbsd had an implementation of strlcpy? Would be safer > to > have a compile test to verify libbsd has it. Hi, 0.0 from 10+ years ago has it, so I think we are safe :-) https://gitlab.freedesktop.org/libbsd/libbsd/blob/0.0/include/bsd/string.h#L34
On Mon, 29 Oct 2018 10:46:50 +0000 Luca Boccassi <bluca@debian.org> wrote: > If libc does not provide strlcpy check for libbsd with pkg-config to > avoid relying on inline version. > > Signed-off-by: Luca Boccassi <bluca@debian.org> > --- > This allows distro maintainers to be able to choose to reduce > duplication and let this code be maintained in one place, in the > external library. > I like the idea, but it causes warnings on Debian testing, and maybe other distros. ipnetns.c:2: warning: "_ATFILE_SOURCE" redefined #define _ATFILE_SOURCE In file included from /usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33, from /usr/include/string.h:26, from /usr/include/bsd/string.h:30, from <command-line>: /usr/include/features.h:326: note: this is the location of the previous definition # define _ATFILE_SOURCE 1 Please figure out how to handle this and resubmit. SUSE open build service might also work to test multiple distro's
On Wed, 2018-10-31 at 08:09 -0700, Stephen Hemminger wrote: > On Mon, 29 Oct 2018 10:46:50 +0000 > Luca Boccassi <bluca@debian.org> wrote: > > > If libc does not provide strlcpy check for libbsd with pkg-config > > to > > avoid relying on inline version. > > > > Signed-off-by: Luca Boccassi <bluca@debian.org> > > --- > > This allows distro maintainers to be able to choose to reduce > > duplication and let this code be maintained in one place, in the > > external library. > > > > I like the idea, but it causes warnings on Debian testing, and maybe > other distros. > > ipnetns.c:2: warning: "_ATFILE_SOURCE" redefined > #define _ATFILE_SOURCE > > In file included from /usr/include/x86_64-linux-gnu/bits/libc-header- > start.h:33, > from /usr/include/string.h:26, > from /usr/include/bsd/string.h:30, > from <command-line>: > /usr/include/features.h:326: note: this is the location of the > previous definition > # define _ATFILE_SOURCE 1 > > > Please figure out how to handle this and resubmit. SUSE open build > service might > also work to test multiple distro's Ah missed that. That happens because features.h defines _ATFILE_SOURCE to 1, but ip/ipnetns.c defines it without a value. According to the spec either way doesn't change the result. This happens because of the quick hack of using -include /usr/include/bsd/string.h which was, well, a quick hack and didn't require to add the include manually everywhere strlcpy was used, even in the future. But it has side effects like this. So I'll send v2 with a less hacky fix, which means defining HAVE_LIBBSD in configure and doing #ifdef HAVE_LIBBSD #include <bsd/string.h> in every file. It also means that this needs to be done for every future use of strlcpy, or the build with libbsd will break. If you or David prefer the hacky way, I can instead send a v3 that does the quick hack, and also changes _ATFILE_SOURCE to 1 so that there is no complaint from the compiler, as the values will be the same.
diff --git a/configure b/configure index 744d6282..1dd9ce84 100755 --- a/configure +++ b/configure @@ -330,8 +330,16 @@ EOF then echo "no" else - echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG - echo "yes" + if ${PKG_CONFIG} libbsd --exists + then + echo 'CFLAGS += -include' `${PKG_CONFIG} libbsd --variable=includedir`'/bsd/string.h' \ + `${PKG_CONFIG} libbsd --cflags` >>$CONFIG + echo 'LDLIBS +=' `${PKG_CONFIG} libbsd --libs` >> $CONFIG + echo "no" + else + echo 'CFLAGS += -DNEED_STRLCPY' >>$CONFIG + echo "yes" + fi fi rm -f $TMPDIR/strtest.c $TMPDIR/strtest }
If libc does not provide strlcpy check for libbsd with pkg-config to avoid relying on inline version. Signed-off-by: Luca Boccassi <bluca@debian.org> --- This allows distro maintainers to be able to choose to reduce duplication and let this code be maintained in one place, in the external library. configure | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)