Re: [PATCH RFC 03/11] printversion: Fix unused variable

[Mark Wielaard]

Hi Ilya (CC Frank),

On Mon, Feb 06, 2023 at 11:25:05PM +0100, Ilya Leoshkevich via Elfutils-devel 
wrote:
> clang complains:
> 
>     debuginfod.cxx:354:1: error: unused variable 'apba__' 
> [-Werror,-Wunused-const-variable]
>     ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
>     ^
>     ../lib/printversion.h:47:21: note: expanded from macro 
> 'ARGP_PROGRAM_BUG_ADDRESS_DEF'
>       const char *const apba__ __asm ("argp_program_bug_address")
>                         ^
> 
> This is as expected: it's used by argp via the
> "argp_program_bug_address" name, which is not visible on the C level.
> Add __attribute__ ((used)) to make sure that the compiler emits it.

Actually I think it found a real issue. Note that the same construct
is used the C eu tools.  But in debuginfod.cxx it says:

/* Name and version of program.  */
/* ARGP_PROGRAM_VERSION_HOOK_DEF = print_version; */ // not this simple for C++

/* Bug report address.  */
ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;

Note how ARGP_PROGRAM_VERSION_HOOK_DEF is commented out and in main it
has:

   /* Parse and process arguments.  */
   int remaining;
   argp_program_version_hook = print_version; // this works
   (void) argp_parse (&argp, argc, argv, ARGP_IN_ORDER, &remaining, NULL);

So it sets print_version, but not argp_program_bug_address.
And indeed debuginfod --help is missing the bug reporting address.

I don't really know/understand why the printversion.h macro trick doesn't work 
with C++ (symbol mangling?). But we do need at least this patch:

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 4271acf4..0ec326d5 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -4172,6 +4165,7 @@ main (int argc, char *argv[])
   /* Parse and process arguments.  */
   int remaining;
   argp_program_version_hook = print_version; // this works
+  argp_program_bug_address = PACKAGE_BUGREPORT;
   (void) argp_parse (&argp, argc, argv, ARGP_IN_ORDER, &remaining, NULL);
   if (remaining != argc)
       error (EXIT_FAILURE, 0,

Then debuginfod --help will say: Report bugs to
https://sourceware.org/bugzilla.

That of course doesn't help with the -Wunused-const-variable warning.

If we cannot figure out the magic variable naming trick with with C++
then maybe we can just not include printversion.h and do it "by hand"?
(as attached)

Cheers,

Mark

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 4271acf4..1ea90645 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -45,7 +45,6 @@ extern "C" {
 #endif
 
 extern "C" {
-#include "printversion.h"
 #include "system.h"
 }
 
@@ -345,13 +344,11 @@ static const char DEBUGINFOD_SQLITE_CLEANUP_DDL[] =
   ;
 
 
-
-
-/* Name and version of program.  */
-/* ARGP_PROGRAM_VERSION_HOOK_DEF = print_version; */ // not this simple for C++
-
-/* Bug report address.  */
-ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
+extern "C" {
+/* Defined in version.c.  Explicitly declared here because including
+   print_version.h magic variable tricks don't work in C++.  */
+void print_version (FILE *stream, struct argp_state *state);
+}
 
 /* Definitions of arguments for argp functions.  */
 static const struct argp_option options[] =
@@ -4172,6 +4169,7 @@ main (int argc, char *argv[])
   /* Parse and process arguments.  */
   int remaining;
   argp_program_version_hook = print_version; // this works
+  argp_program_bug_address = PACKAGE_BUGREPORT;
   (void) argp_parse (&argp, argc, argv, ARGP_IN_ORDER, &remaining, NULL);
   if (remaining != argc)
       error (EXIT_FAILURE, 0,