diff mbox series

lib: avoid potential problems with `-fno-common`

Message ID 20210827155147.4069858-1-abdulras@google.com
State Committed
Headers
Series lib: avoid potential problems with `-fno-common` |

Commit Message

Saleem Abdulrasool Aug. 27, 2021, 3:51 p.m. UTC 
  This properly homes the fallback function into a translation unit rather
than trying to define an inline common definition for the fallback path.
The intent of the original approach was to actually simply avoid adding
a new source file that is used for the fallback path.  However, that may
cause trouble with multiple definitions if the symbol does not get vague
linkage (which itself is not particularly great).  This simplifies the
behaviour at the cost of an extra inode.
---
 lib/ChangeLog   |  9 +++++++++
 lib/Makefile.am |  2 +-
 lib/error.c     | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/system.h    | 17 ++---------------
 4 files changed, 61 insertions(+), 16 deletions(-)
 create mode 100644 lib/error.c

Comments

Mark Wielaard Aug. 27, 2021, 4:59 p.m. UTC | #1 
Hi Saleem,

On Fri, 2021-08-27 at 15:51 +0000, Saleem Abdulrasool via Elfutils-devel wrote:
> This properly homes the fallback function into a translation unit rather
> than trying to define an inline common definition for the fallback path.
> The intent of the original approach was to actually simply avoid adding
> a new source file that is used for the fallback path.  However, that may
> cause trouble with multiple definitions if the symbol does not get vague
> linkage (which itself is not particularly great).  This simplifies the
> behaviour at the cost of an extra inode.

I applied this, but messed up, sorry.
I somehow dropped the lib/error.c file from the commit.

When the buildbot yells and screams about a broken build that wasn't
your fault. I pushed a followup commit to fix it:

commit 610623458b7e98ed3e912e4b7ca8050f6ce4c698 (HEAD -> master)
Author: Mark Wielaard <mark@klomp.org>
Date:   Fri Aug 27 18:47:30 2021 +0200

    Add lib/error.c
    
    This new file was supposed to be part of 4d6dd0e5a "lib: avoid potential
    problems with `-fno-common`".
    
    Signed-off-by: Mark Wielaard <mark@klomp.org>

Apologies,

Mark
diff mbox series

Patch

diff --git a/lib/ChangeLog b/lib/ChangeLog
index 589953cf..c784b689 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,12 @@ 
+2021-08-23  Saleem Abdulrasool  <abdulras@google.com>
+
+	* system.h: Remove inline definition for error and error_message_count
+	in the fallback path.
+	* Makefile.am (libeu_a_SOURCES): Add error.c.
+	* error.c: New file, moves the previous inline definitions to avoid
+	multiple definitions properly rather than relying on -fcommon and vague
+	linkage.
+
 2021-08-20  Saleem Abdulrasool  <abdulras@google.com>
 
 	* system.h: Check for HAVE_ERROR_H and HAVE_ERR_H and define
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 97bf7329..766fbcd7 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -35,7 +35,7 @@  noinst_LIBRARIES = libeu.a
 
 libeu_a_SOURCES = xstrdup.c xstrndup.c xmalloc.c next_prime.c \
 		  crc32.c crc32_file.c \
-		  color.c printversion.c
+		  color.c error.c printversion.c
 
 noinst_HEADERS = fixedsizehash.h libeu.h system.h dynamicsizehash.h list.h \
 		 eu-config.h color.h printversion.h bpf.h \
diff --git a/lib/error.c b/lib/error.c
new file mode 100644
index 00000000..75e964fd
--- /dev/null
+++ b/lib/error.c
@@ -0,0 +1,49 @@ 
+/* Definitions for error fallback functions.
+   Copyright (C) 2021 Google, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of either
+
+     * the GNU Lesser General Public License as published by the Free
+       Software Foundation; either version 3 of the License, or (at
+       your option) any later version
+
+   or
+
+     * the GNU General Public License as published by the Free
+       Software Foundation; either version 2 of the License, or (at
+       your option) any later version
+
+   or both in parallel, as here.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received copies of the GNU General Public License and
+   the GNU Lesser General Public License along with this program.  If
+   not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+
+#if !defined(HAVE_ERROR_H) && defined(HAVE_ERR_H)
+#include <stdarg.h>
+#include <stdlib.h>
+#include <err.h>
+
+unsigned int error_message_count = 0;
+
+void error(int status, int errnum, const char *format, ...) {
+  va_list argp;
+
+  va_start(argp, format);
+  verr(status, format, argp);
+  va_end(argp);
+
+  if (status)
+    exit(status);
+  ++error_message_count;
+}
+#endif
diff --git a/lib/system.h b/lib/system.h
index b963fd15..edbc8488 100644
--- a/lib/system.h
+++ b/lib/system.h
@@ -45,21 +45,8 @@ 
 #if defined(HAVE_ERROR_H)
 #include <error.h>
 #elif defined(HAVE_ERR_H)
-#include <err.h>
-
-static int error_message_count = 0;
-
-static inline void error(int status, int errnum, const char *format, ...) {
-  va_list argp;
-
-  va_start(argp, format);
-  verr(status, format, argp);
-  va_end(argp);
-
-  if (status)
-    exit(status);
-  ++error_message_count;
-}
+extern int error_message_count;
+void error(int status, int errnum, const char *format, ...);
 #else
 #error "err.h or error.h must be available"
 #endif