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.