[Libguestfs] [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module

[Jim Meyering]

Richard W.M. Jones wrote:

> On Mon, Aug 24, 2009 at 04:26:16PM +0200, Jim Meyering wrote:
>> Richard W.M. Jones wrote:
>>
>> > On Mon, Aug 24, 2009 at 02:21:51PM +0200, Jim Meyering wrote:
>> >> Sure.  That works, too.
>> >> Here's the incremental:
>> >
>> > No, this doesn't fix the path munging to use real_argv0.
>>
>> "fix"?
>> Is something incorrect, or is it just that you'd prefer
>> to move the definition of real_argv0 "up"?
>>
>> I see only two uses of argv[0] in main:
>>
>> - path-related stuff *before* I change argv[0] and define real_argv0
>>
>>   if (getenv ("LIBGUESTFS_PATH") == NULL &&
>>       argv[0] &&
>>       (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
>>     guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
>>
>> - then, this code, afterwards, that I did change to use real_argv0:
>>
>>     strcpy (cmd, "a=`virt-inspector");
>>     while (optind < argc) {
>>       if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
>>     ...
>>     sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
>>
>> If you'd prefer to move the definition of real_argv0 to precede
>> the former, let me know, and I'll move it and adjust.
>> However, note that that has no impact on correctness.
>
> You're right - for some reason I thought the path-munging code
> occurred after overwriting argv[0].
>
> In general though I think the code is now much more prone to errors if
> we rearrange the code in future.  (It's already complex and prone to
> introducing errors).
>
> If the only benefit to rewriting argv[0] is that we get a slightly
> better error message from getopt, then I'm inclined not to do it at
> all.  Leave all the argv[0] code as it is, and change anything else to
> use program_name.

There are a couple of compelling arguments for making the
change in general:

  - testing and reproducibility: with the change, all diagnostics
    start with the constant string, "program_name: ".
    Otherwise, you'll have most that are that way, yet a few
    that then start with an absolute directory name and end in
    "/.libs/lt-program_name: ".  Worse still, in some environments,
    that leading directory name will vary from run to run depending
    on mount point names.

  - overall uniformity.  in building the tools and testing them, we've
    seen how much output you may have to wade through when diagnosing a
    problem.  If we ensure that all diagnostics start with "program_name: "
    it's that much easier to be sure that a quick use of grep finds all
    error messages.

If it will make it easier to accept, I will add some small tests
that verify the expected behavior.

An alternative is not to modify argv at all, but
rather to make a copy of it, change argv_copy[0],
and to pass argv_copy to getopt_long.

Let me know.