[PATCH] Check for existence of mempcpy

[PATCH] Check for existence of mempcpy

[Ulf Hermann]

If it doesn't exist, provide a definition based on memcpy.

Signed-off-by: Ulf Hermann <ulf.hermann@qt.io>
---
 ChangeLog                      | 4 ++++
 configure.ac                   | 3 +++
 lib/ChangeLog                  | 5 +++++
 lib/system.h                   | 5 +++++
 lib/xstrndup.c                 | 2 +-
 libasm/ChangeLog               | 4 ++++
 libasm/disasm_str.c            | 2 +-
 libdwfl/ChangeLog              | 4 ++++
 libdwfl/linux-kernel-modules.c | 1 +
 libebl/ChangeLog               | 5 +++++
 libebl/eblmachineflagname.c    | 1 +
 libebl/eblopenbackend.c        | 1 +
 tests/ChangeLog                | 4 ++++
 tests/elfstrmerge.c            | 1 +
 14 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b50f79e..48185c3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
+
+	* configure.ac: Add check for mempcpy.
+
 2017-02-09  Mark Wielaard  <mark@klomp.org>
 
 	* configure.ac: Add check for adding -D_FORTIFY_SOURCE=2 to CFLAGS.
diff --git a/configure.ac b/configure.ac
index 46055f2..3c35dac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -285,6 +285,9 @@ AC_CHECK_DECLS([memrchr, rawmemchr],[],[],
                [#define _GNU_SOURCE
                 #include <string.h>])
 AC_CHECK_DECLS([powerof2],[],[],[#include <sys/param.h>])
+AC_CHECK_DECLS([mempcpy],[],[],
+               [#define _GNU_SOURCE
+                #include <string.h>])
 
 AC_CHECK_LIB([stdc++], [__cxa_demangle], [dnl
 AC_DEFINE([USE_DEMANGLE], [1], [Defined if demangling is enabled])])
diff --git a/lib/ChangeLog b/lib/ChangeLog
index 6578ddb..fd63e03 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,5 +1,10 @@
 2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
 
+	* system.h: Provide mempcpy if it doesn't exist.
+	* xstrndup.c: Include system.h.
+
+2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
+
 	* crc32_file.c: Use _SC_PAGESIZE rather than _SC_PAGE_SIZE.
 
 2017-02-14  Ulf Hermann  <ulf.hermann@qt.io>
diff --git a/lib/system.h b/lib/system.h
index dde7c4a..429b0c3 100644
--- a/lib/system.h
+++ b/lib/system.h
@@ -68,6 +68,11 @@
 #define powerof2(x) (((x) & ((x) - 1)) == 0)
 #endif
 
+#if !HAVE_DECL_MEMPCPY
+#define mempcpy(dest, src, n) \
+    ((void *) ((char *) memcpy (dest, src, n) + (size_t) n))
+#endif
+
 /* A special gettext function we use if the strings are too short.  */
 #define sgettext(Str) \
   ({ const char *__res = strrchr (gettext (Str), '|');			      \
diff --git a/lib/xstrndup.c b/lib/xstrndup.c
index d43e3b9..a257aa9 100644
--- a/lib/xstrndup.c
+++ b/lib/xstrndup.c
@@ -33,7 +33,7 @@
 #include <stdint.h>
 #include <string.h>
 #include "libeu.h"
-
+#include "system.h"
 
 /* Return a newly allocated copy of STRING.  */
 char *
diff --git a/libasm/ChangeLog b/libasm/ChangeLog
index fe06007..26fc5a9 100644
--- a/libasm/ChangeLog
+++ b/libasm/ChangeLog
@@ -1,3 +1,7 @@
+2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
+
+	* disasm_str.c: Include system.h.
+
 2015-10-11  Akihiko Odaki  <akihiko.odaki.4i@stu.hosei.ac.jp>
 
 	* asm_align.c: Remove sys/param.h include.
diff --git a/libasm/disasm_str.c b/libasm/disasm_str.c
index 5b0bb29..c14e6d5 100644
--- a/libasm/disasm_str.c
+++ b/libasm/disasm_str.c
@@ -31,7 +31,7 @@
 #endif
 
 #include <string.h>
-
+#include <system.h>
 #include "libasmP.h"
 
 
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 52466cc..4c9f4f6 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,5 +1,9 @@
 2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
 
+	* linux-kernel-modules.c: Include system.h.
+
+2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
+
 	* linux-kernel-modules.c: Use sysconf(_SC_PAGESIZE) to get page size.
 	* linux-proc-maps.c: Likewise.
 
diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
index a7ac19d..7345e76 100644
--- a/libdwfl/linux-kernel-modules.c
+++ b/libdwfl/linux-kernel-modules.c
@@ -34,6 +34,7 @@
 #endif
 
 #include <config.h>
+#include <system.h>
 
 #include "libdwflP.h"
 #include <inttypes.h>
diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index 0560c6a..719d08d 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,3 +1,8 @@
+2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
+
+	* eblmachineflagname.c: Include system.h.
+	* eblopenbackend.c: Likewise.
+
 2016-07-08  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (gen_SOURCES): Remove eblstrtab.c.
diff --git a/libebl/eblmachineflagname.c b/libebl/eblmachineflagname.c
index 6079a61..5f44077 100644
--- a/libebl/eblmachineflagname.c
+++ b/libebl/eblmachineflagname.c
@@ -33,6 +33,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <system.h>
 #include <libeblP.h>
 
 
diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c
index aa75b95..f3a65cf 100644
--- a/libebl/eblopenbackend.c
+++ b/libebl/eblopenbackend.c
@@ -39,6 +39,7 @@
 #include <string.h>
 #include <stdio.h>
 
+#include <system.h>
 #include <libeblP.h>
 
 
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 71dba42..4af450c 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2017-02-15  Ulf Hermann  <ulf.hermann@qt.io>
+
+	* elfstrmerge.c: Include system.h.
+
 2017-02-13  Ulf Hermann  <ulf.hermann@qt.io>
 
 	* Makefile.am: Add test for unwinding with frame pointers on aarch64
diff --git a/tests/elfstrmerge.c b/tests/elfstrmerge.c
index c2c3fb9..8d5b53c 100644
--- a/tests/elfstrmerge.c
+++ b/tests/elfstrmerge.c
@@ -29,6 +29,7 @@
 #include <inttypes.h>
 #include <unistd.h>
 
+#include <system.h>
 #include <gelf.h>
 #include ELFUTILS_HEADER(dwelf)
 #include "elf-knowledge.h"
-- 
2.1.4

Re: [PATCH] Check for existence of mempcpy

[Mark Wielaard]

On Thu, 2017-02-16 at 10:10 +0100, Ulf Hermann wrote:
> If it doesn't exist, provide a definition based on memcpy.

Applied, but slightly reluctantly. I have no way to test this. And it
will evaluate the last argument (n) twice. Which seems to not matter in
the current calls in our codebase. But it might subtly break something
if someone forgets.

Cheers,

Mark

Re: [PATCH] Check for existence of mempcpy

[Ulf Hermann]

On 02/17/2017 10:46 AM, Mark Wielaard wrote:
> On Thu, 2017-02-16 at 10:10 +0100, Ulf Hermann wrote:
>> If it doesn't exist, provide a definition based on memcpy.
> 
> Applied, but slightly reluctantly. I have no way to test this. And it
> will evaluate the last argument (n) twice. Which seems to not matter in
> the current calls in our codebase. But it might subtly break something
> if someone forgets.

True. With other missing functions I tend to conditionally build the replacements into libeu.a as actual functions. That requires libeu.a to be linked into libdw.so, libelf.so, etc. I will create a followup patch that does the same with mempcpy.

Ulf

Re: [PATCH] Check for existence of mempcpy

[Mark Wielaard]

On Fri, 2017-02-17 at 10:50 +0100, Ulf Hermann wrote:
> True. With other missing functions I tend to conditionally build the
> replacements into libeu.a as actual functions. That requires libeu.a
> to be linked into libdw.so, libelf.so, etc. I will create a followup
> patch that does the same with mempcpy.

If at all possible I would like elfutils to not turn into some
abstraction layer for broken non-GNU/Linux systems. I don't mind small
(mostly) obvious correct defines/checks or tweaks to help out people
using such broken systems. But if we need a lot more of these things I
think we should look if we can just import the necessary gnulib modules
to get portability functions.

Cheers,

Mark

Re: [PATCH] Check for existence of mempcpy

[Ulf Hermann]

> If at all possible I would like elfutils to not turn into some
> abstraction layer for broken non-GNU/Linux systems. I don't mind small
> (mostly) obvious correct defines/checks or tweaks to help out people
> using such broken systems. But if we need a lot more of these things I
> think we should look if we can just import the necessary gnulib modules
> to get portability functions.

I've actually done that, but the amount of stuff that would need to be imported is staggering (see for example https://codereview.qt-project.org/#/c/182654/13) and comes with a serious mess of different licenses.

I've since switched to a different strategy where I rather disable features than import large gnulib modules. Disabling maintainer mode (for obstack) and any code that uses argp certainly helps. The remaining pain point is tsearch/tfind/tdelete/tdestroy for which we probably need third party code. Another problem is that we need fts for dwfl_linux_kernel_*. I'm on the edge on whether to include the gnulib implementation or disable it.

The retrying I/O functions can be implemented on top of either Linux/GNU or windows API and I would just move them to a separate file which I can replace based on availability of one or the other (I don't need to upstream the windows implementation). This technique will require universally using the retrying variants rather than the plain ones, which is a good thing anyway.

Replacements for mmap() and dlopen() on windows are a somewhat more involved matter but I don't really need to upstream them either, if you don't want them.

All other missing functions are just a few lines of C code each, which can either be pulled from gnulib or reimplemented. As I didn't want to taint myself with license problems I chose to reimplement them for now. But that can be decided on a case by case basis. Those are the patches I'm going to focus on next.

And then there are nonstandard C constructs, especially void* arithmetics and statement expressions, which I would like to replace with standards compliant C. That is a somewhat larger and not yet finished project, and I'm hoping you're generally open to it.

cheers,
Ulf

Re: [PATCH] Check for existence of mempcpy

[Mark Wielaard]

On Fri, Feb 17, 2017 at 01:16:11PM +0100, Ulf Hermann wrote:
> And then there are nonstandard C constructs, especially void* arithmetics

void * arithmetic can at times be surprising, so that seems a fine cleanup.
Best to make sure the code compiles cleanly with -Wpointer-arith.

> and statement expressions, which I would like to replace with standards
> compliant C. That is a somewhat larger and not yet finished project, and
> I'm hoping you're generally open to it.

Sure, but it depends on the result. Avoiding void * arithmetic seems kind
of worth it because it is sometimes surprising and it makes sure we always
use the correct type for things involving pointer arithmetic. Which probably
cleans up the code. But statement expressions are really useful to define
safe macros without having to turn everything into a nested or inlined
function. So I am not sure such a specific change would result in cleaner
code. So I would have to see the result first. Alternatively we could also
just extend the configure test that checks the compiler supports proper
GNU99 by adding a statement expression to the testcase (we already expect
support for mixed declarations and code, nested functions and arrays of
variable lengths because those are really useful).

Cheers,

Mark