From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 4C4CF3858D33 for ; Tue, 7 Feb 2023 20:44:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4C4CF3858D33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: by gnu.wildebeest.org (Postfix, from userid 1000) id 60FA23000A21; Tue, 7 Feb 2023 21:44:26 +0100 (CET) Date: Tue, 7 Feb 2023 21:44:26 +0100 From: Mark Wielaard To: Ilya Leoshkevich Cc: elfutils-devel@sourceware.org, fche@elastic.org Subject: Re: [PATCH RFC 03/11] printversion: Fix unused variable Message-ID: <20230207204426.GD25444@gnu.wildebeest.org> References: <20230206222513.1773039-1-iii@linux.ibm.com> <20230206222513.1773039-4-iii@linux.ibm.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="M9NhX3UHpAaciwkO" Content-Disposition: inline In-Reply-To: <20230206222513.1773039-4-iii@linux.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-3037.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="debuginfod-version.patch" 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, --M9NhX3UHpAaciwkO--