libXfont2 strlcat/strlcpy patch

Submitted by Guillem Jover on Nov. 20, 2019, 12:52 a.m.

Details

Message ID 20191120005223.GA1545@gaara.hadrons.org
State New
Headers show
Series "libXfont2 strlcat/strlcpy patch" ( rev: 1 ) in X.org

Not browsing as part of any series.

Commit Message

Guillem Jover Nov. 20, 2019, 12:52 a.m.
Hi!

On Sun, 2019-11-03 at 18:08:00 +0100, Roman Gilg wrote:
> On Wed, Oct 23, 2019 at 5:33 PM Alan Coopersmith wrote:
> > On 10/23/19 12:48 AM, Michael Joost wrote:
> > > regarding your recent patch in xorg/libXfont2 (strlcat/strlcpy fallbacks) I ran into a problem on Linux while having libbsd installed. The configure correctly detects libbsd and those functions, however, the macro HAVE_LIBBSD is never defined in this process. The compile then fails with "implicit declaration" error, as the libbsd includes are not loaded from replace.h. When defining this macro manually, everything is fine.
> > > I think HAVE_LIBBSD should be defined by configure when it has found -lbsd.
> >
> > Hmm, I see what you mean, but I've not heard of problems from any other Linux
> > users/distro builders, so perhaps something else is masking it for them?
> >
> > I'm cc'ing the list to see if those who work on Linux see the problem, or
> > want to provide a patch that they can test more easily than I can.
> 
> I don't know what needs to be done here, but just wanted to say that I
> experienced strlcat, strlcpy problems when compiling libXfont master
> branch on Arch as well.
> 
> I can not build it from commit 2178c7445a on.

The attached patch should fix this. I'm not sure whether it would be
better to put the LIBBSD flags into the OS ones, but that might
require more code shuffling.

Thanks,
Guillem

Patch hide | download patch | download mbox

From 68f0f226e2209539290e0402b15cddbe32fe7f93 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@hadrons.org>
Date: Wed, 20 Nov 2019 01:42:52 +0100
Subject: [PATCH libXfont] build: Fix libbsd compatibility layer

Switch to use the libbsd-overlay, which implies less changes to the
actual source code. Get the flags from pkg-config, and apply them during
all header and compatibility checks.

Signed-off-by: Guillem Jover <guillem@hadrons.org>
---
 Makefile.am        |  4 ++--
 configure.ac       | 16 ++++++++++++----
 src/util/replace.h |  6 ------
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 5af2e23..8732c1e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -49,7 +49,7 @@  pkgconfig_DATA = xfont2.pc
 lib_LTLIBRARIES = libXfont2.la
 
 AM_CPPFLAGS = -I${top_srcdir}/include
-AM_CFLAGS = $(XFONT_CFLAGS) $(OS_CFLAGS) $(CWARNFLAGS)
+AM_CFLAGS = $(XFONT_CFLAGS) $(LIBBSD_CFLAGS) $(OS_CFLAGS) $(CWARNFLAGS)
 
 libXfont2_la_SOURCES =			\
 	src/stubs/atom.c		\
@@ -66,7 +66,7 @@  libXfont2_la_SOURCES =			\
 
 libXfont2_la_LDFLAGS = -version-number 2:0:0 -no-undefined
 
-libXfont2_la_LIBADD = $(Z_LIBS) $(MATH_LIBS) $(XFONT_LIBS) $(LTLIBOBJS)
+libXfont2_la_LIBADD = $(LIBBSD_LIBS) $(Z_LIBS) $(MATH_LIBS) $(XFONT_LIBS) $(LTLIBOBJS)
 
 if XFONT_FONTFILE
 libXfont2_la_SOURCES +=			\
diff --git a/configure.ac b/configure.ac
index e497325..7cc2fce 100644
--- a/configure.ac
+++ b/configure.ac
@@ -47,21 +47,29 @@  XORG_WITH_XMLTO(0.0.22)
 XORG_WITH_FOP
 XORG_CHECK_SGML_DOCTOOLS(1.7)
 
+# If the first PKG_CHECK_MODULES appears inside a conditional, pkg-config
+# must first be located explicitly.
+PKG_PROG_PKG_CONFIG
+
+PKG_CHECK_MODULES(LIBBSD, [libbsd-overlay])
+save_CFLAGS="$CFLAGS"
+save_LIBS="$LIBS"
+CFLAGS="$CFLAGS $LIBBSD_CFLAGS"
+LIBS="$LIBS $LIBBSD_LIBS"
+
 # Checks for header files.
 AC_CHECK_HEADERS([endian.h poll.h sys/poll.h])
 
 # Checks for library functions.
 AC_CHECK_FUNCS([poll readlink])
-AC_SEARCH_LIBS([strlcat], [bsd])
 AC_CONFIG_LIBOBJ_DIR([src/util])
 AC_REPLACE_FUNCS([reallocarray realpath strlcat strlcpy])
 
 # Check for BSDish err.h
 AC_CHECK_HEADERS([err.h])
 
-# If the first PKG_CHECK_MODULES appears inside a conditional, pkg-config
-# must first be located explicitly.
-PKG_PROG_PKG_CONFIG
+CFLAGS="$save_CFLAGS"
+LIBS="$save_LIBS"
 
 #
 # select libraries to include
diff --git a/src/util/replace.h b/src/util/replace.h
index 53a81a2..367a1fd 100644
--- a/src/util/replace.h
+++ b/src/util/replace.h
@@ -32,9 +32,6 @@ 
 #include <X11/Xfuncproto.h>
 
 #include <stdlib.h>
-#if defined(HAVE_LIBBSD) && defined(HAVE_REALLOCARRAY)
-#include <bsd/stdlib.h>       /* for reallocarray */
-#endif
 
 #ifndef HAVE_REALLOCARRAY
 extern _X_HIDDEN void *
@@ -46,9 +43,6 @@  reallocarray(void *optr, size_t nmemb, size_t size);
 #endif
 
 #include <string.h>
-#if defined(HAVE_LIBBSD) && defined(HAVE_STRLCPY)
-#include <bsd/string.h>       /* for strlcpy, strlcat */
-#endif
 
 #ifndef HAVE_STRLCPY
 extern _X_HIDDEN size_t
-- 
2.24.0.432.g9d3f5f5b63


Comments

On Wed, Nov 20, 2019 at 1:53 AM Guillem Jover <guillem@hadrons.org> wrote:
>
> Hi!
>
> On Sun, 2019-11-03 at 18:08:00 +0100, Roman Gilg wrote:
> > On Wed, Oct 23, 2019 at 5:33 PM Alan Coopersmith wrote:
> > > On 10/23/19 12:48 AM, Michael Joost wrote:
> > > > regarding your recent patch in xorg/libXfont2 (strlcat/strlcpy fallbacks) I ran into a problem on Linux while having libbsd installed. The configure correctly detects libbsd and those functions, however, the macro HAVE_LIBBSD is never defined in this process. The compile then fails with "implicit declaration" error, as the libbsd includes are not loaded from replace.h. When defining this macro manually, everything is fine.
> > > > I think HAVE_LIBBSD should be defined by configure when it has found -lbsd.
> > >
> > > Hmm, I see what you mean, but I've not heard of problems from any other Linux
> > > users/distro builders, so perhaps something else is masking it for them?
> > >
> > > I'm cc'ing the list to see if those who work on Linux see the problem, or
> > > want to provide a patch that they can test more easily than I can.
> >
> > I don't know what needs to be done here, but just wanted to say that I
> > experienced strlcat, strlcpy problems when compiling libXfont master
> > branch on Arch as well.
> >
> > I can not build it from commit 2178c7445a on.
>
> The attached patch should fix this. I'm not sure whether it would be
> better to put the LIBBSD flags into the OS ones, but that might
> require more code shuffling.

Indeed this patch fixes it. Can build with it applied on top of master.

Tested-by: Roman Gilg <subdiff@gmail.com>

> Thanks,
> Guillem