[help-gengetopt] Re: GNU Gengetopt 2.22

[Andre Noll]

On 21:07, Lorenzo Bettini wrote:
> I'm responding in the mailing list help-gengetopt (you're subscribed to 
> it, aren't you?)

Sure I am.

> >Similarly, if gengetopt is compiled on Mac, and then configured and
> >built on Linux, it results in
> >
> >     argsdef.o: file not recognized: File format not recognized
> >     
> >However, this is not a serious issue because "make clean" solves the
> >problem.
> >
> 
> mh... I don't think this is the expected behavior: issueing another 
> configure might change the config.h and the files that include it will 
> be recompiled, but not all files are expected to include it; actually if 
> you build a package for different platforms you should run each 
> configure in a separate directory (one for each different configure).

OK. Another solution would be to let all object files depend on Makefile
which is also generated by configure.

> >Finally, when compiling audiod.cmdline.c, a file generated by
> >gengetopt-2.22, I see the following warning, which didn't show up
> >when using earlier versions of gengetopt:
> >
> >audiod.cmdline.c: In function 'audiod_cmdline_parser_release':
> >audiod.cmdline.c:337: warning: dereferencing type-punned pointer will 
> >break strict-aliasing rules
> >
> >The offending line 337 in audiod.cmdline.c is
> >
> >       free_multiple_field (args_info->user_allow_given, (void 
> >       **)&(args_info->user_allow_arg), &(args_info->user_allow_orig));
> >
> >And free_multiple_field() looks like this:
> >
> >static void
> >free_multiple_field(unsigned int len, void **arg, char ***orig)
> >{
> >  unsigned int i;
> >  if (*arg) {
> >    for (i = 0; i < len; ++i)
> >      {
> >        free_string_field(&((*orig)[i]));
> >      }
> >
> >    free (*arg);
> >    *arg = 0;
> >    free (*orig);
> >    *orig = 0;
> >  }
> >}
> >
> >Why you are using void ** as the second parameter "arg" when the function
> >is only using *arg? Wouldn't it be better to generate
> >
> >static void
> >free_multiple_field(unsigned int len, void *arg, char ***orig)
> >{
> >  unsigned int i;
> >  if (arg) {
> >    for (i = 0; i < len; ++i)
> >      {
> >        free_string_field(&((*orig)[i]));
> >      }
> >
> >    free (arg);
> >    free (*orig);
> >    *orig = 0;
> >  }
> >}
> >
> >instead? This would allow to get rid of the cast in the offending line.
> >The only drawback I can see is that args_info->user_allow_arg is not
> >set to NULL by free_multiple_field() when using this approach.
> 
> mh... setting that arg to null is crucial (since what is freed could be 
> re-used internally), but probably I could set it to NULL after that 
> function call, I'll take a look at this.

I see. An alternative for getting rid of this warning is to introduce an
intermediate (void *) cast:

        free_multiple_field (args_info->user_allow_given, (void **)(void 
*)&(args_info->user_allow_arg), &(args_info->user_allow_orig));
                                                                   ^^^^^^^^
> Actually, I don't get this warning, so thanks for reporting it.

You need to compile with -Wstrict-aliasing (included in -Wall) and
with -fstrict-aliasing (enabled at levels -O2, -O3, -Os).

Have fun
Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

signature.asc
Description: Digital signature