[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Richard W.M. Jones wrote:
> On Mon, Aug 24, 2009 at 12:21:54PM +0200, Jim Meyering wrote:
>> +  /* getopt_long uses argv[0], so give it the sanitized name, too.  */
>> +  argv[0] = bad_cast (program_name);
>
> This is wrong.
>
> We rely on seeing the original argv[0] later in the code, so that
> people can run ./fish/guestfish from the source directory and have the
> appliance path set correctly:
>
>   /* If developing, add ./appliance to the path.  Note that libtools
>    * interferes with this because uninstalled guestfish is a shell
>    * script that runs the real program with an absolute path.  Detect
>    * that too.
>    *
>    * BUT if LIBGUESTFS_PATH environment variable is already set by
>    * the user, then don't override it.
>    */
>   if (getenv ("LIBGUESTFS_PATH") == NULL &&
>       argv[0] &&
>       (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
>     guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);

Good catch.  I'll adapt by saving argv[0] just before calling
getopt_long, setting it as above, and then restoring the original,
afterwards.

However, bear in mind that determining the absolute name of
the running program is quite a portability rats nest, in general.
The above won't always work.

Also, it is good policy, IMHO, to make it so a program works the same
way no matter where the actual binary happens to be located in a file
system hierarchy.

>> +  enum { HELP_OPTION = CHAR_MAX + 1 };
>> +
>>    static const char *options = "a:Df:h::im:nrv?Vx";
>>    static const struct option long_options[] = {
>>      { "add", 1, 0, 'a' },
>>      { "cmd-help", 2, 0, 'h' },
>>      { "file", 1, 0, 'f' },
>> -    { "help", 0, 0, '?' },
>> +    { "help", 0, 0, HELP_OPTION },
>
> I'm unclear on why we do this - what's wrong with using '?'?

So that we can distinguish getop-diagnosed failure
(where it returns '?') from --help.
With my patch, only --help prints that output, now.
getopt-diagnosed failures get an abbreviated diagnostic like this:

    $ ./guestfish --mount
    guestfish: option '--mount' requires an argument
    Try `guestfish --help' for more information.
    [Exit 1]


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]