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]