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

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Thu, 2023-02-09 at 15:04 +0100, Mark Wielaard wrote:
> Hi Ilya,
> 
> On Wed, 2023-02-08 at 13:22 +0100, Ilya Leoshkevich wrote:
> > If I build:
> > 
> > const char *const apba__ __asm ("argp_program_bug_address") \
> > __attribute__ ((used)) = "foobarbaz";
> > 
> > with C and C++, the difference is going to be:
> > 
> > @@ -1,6 +1,5 @@
> >         .file   "1.c"
> >         .text
> > -       .globl  argp_program_bug_address
> >         .section        .rodata.str1.1,"aMS",@progbits,1
> >  .LC0:
> >         .string "foobarbaz"
> > 
> > This must have to do with C and C++ standards treating const
> > differently [1]. The solution is to add extern:
> > 
> > --- a/lib/printversion.h
> > +++ b/lib/printversion.h
> > @@ -44,6 +44,7 @@ void print_version (FILE *stream, struct
> > argp_state
> > *state);
> >    void (*const apvh) (FILE *, struct argp_state *) \
> >     __asm ("argp_program_version_hook")
> >  #define ARGP_PROGRAM_BUG_ADDRESS_DEF \
> > +  extern const char *const apba__; \
> >    const char *const apba__ __asm ("argp_program_bug_address") \
> >    __attribute__ ((used))
> > 
> > I can include this in v2 if it works for you.
> > 
> > [1]
> > https://stackoverflow.com/questions/8908071/const-correctness-in-c-vs-c
> 
> O nice, that explains it. But then in that case I don't think you
> need
> the __attribute__ ((used)) anymore.
> 
> Also as a nitpick the multiline define could be just a single line if
> you declare the extern on its own in printversion.h.
> 
> And it would be nice to also cleanup apvh/argp_program_version_hook
> so
> it too works with c++, so we can remove the hack in debuginfod.cxx.
> 
> Does the following work for you?
> 
> diff --git a/lib/printversion.h b/lib/printversion.h
> index a9e059ff..bc9ca7ae 100644
> --- a/lib/printversion.h
> +++ b/lib/printversion.h
> @@ -40,9 +40,11 @@ void print_version (FILE *stream, struct
> argp_state *state);
>     variables as non-const (which is correct in general).  But we can
>     do better, it is not going to change.  So we want to move them
> into
>     the .rodata section.  Define macros to do the trick.  */
> +extern void (*const apvh) (FILE *, struct argp_state *);
>  #define ARGP_PROGRAM_VERSION_HOOK_DEF \
>    void (*const apvh) (FILE *, struct argp_state *) \
>     __asm ("argp_program_version_hook")
> +extern const char *const apba__;
>  #define ARGP_PROGRAM_BUG_ADDRESS_DEF \
>    const char *const apba__ __asm ("argp_program_bug_address")
>  
> diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
> index 4271acf4..99b1f2b9 100644
> --- a/debuginfod/debuginfod.cxx
> +++ b/debuginfod/debuginfod.cxx
> @@ -348,7 +348,7 @@ static const char DEBUGINFOD_SQLITE_CLEANUP_DDL[]
> =
>  
>  
>  /* Name and version of program.  */
> -/* ARGP_PROGRAM_VERSION_HOOK_DEF = print_version; */ // not this
> simple for C++
> +ARGP_PROGRAM_VERSION_HOOK_DEF = print_version;
>  
>  /* Bug report address.  */
>  ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
> @@ -4171,7 +4171,6 @@ main (int argc, char *argv[])
>  
>    /* 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);
>    if (remaining != argc)
>        error (EXIT_FAILURE, 0,
> 
> Thanks,
> 
> Mark

This works for me, I will add this to v3. Thanks!