GNOME Bugzilla – Bug 642825
Unnecessary assertion failure in g_option_context_parse()
Last modified: 2012-02-26 20:22:06 UTC
Overview: G_OPTION_FLAG_OPTIONAL_ARG confuses people who don't read the description well enough. See the closed bug 577727 and bug 588989. If G_OPTION_FLAG_OPTIONAL_ARG is set but is not combined with G_OPTION_ARG_CALLBACK the program reacts very differently to a command with a short option and a command with a long option. Short option: G_OPTION_FLAG_OPTIONAL_ARG is ignored. Long option: Assertion failure. It would be better to consistently ignore irrelevant G_OPTION_FLAGs. The source code suggests that someone has had the ambition to do just that, but not quite succeeded. Steps to reproduce: Use the test case in bug 577727 comment 3. ./testoption -s ./testoption --string Actual results: Missing argument for -s GLib:ERROR:goption.c:1138:parse_arg: assertion failed: (value || OPTIONAL_ARG (entry) || NO_ARG (entry)) Avbruten (SIGABRT) Expected results: Missing argument for -s Missing argument for --string Build date and platform: Ubuntu 10.10, source code of glib, etc. built with jhbuild on 2011-02-17. Additional information: Two possible improvements of goption.c: 1. In parse_long_option(), replace if (!(group->entries[j].flags & G_OPTION_FLAG_OPTIONAL_ARG)) by if (!OPTIONAL_ARG (&group->entries[j])) and replace else if (*idx >= *argc - 1 && group->entries[j].flags & G_OPTION_FLAG_OPTIONAL_ARG) by else if (*idx >= *argc - 1 && OPTIONAL_ARG (&group->entries[j])) (Compare parse_short_option().) 2. An even better improvement would be to add more tests in g_option_group_add_entries(), printing warnings and removing erroneous flags. For instance something like this in the loop where short_name is tested: if (group->entries[i].arg != G_OPTION_ARG_NONE && (group->entries[i].flags & G_OPTION_FLAG_REVERSE)) { g_warning(.....); group->entries[i].flags &= ~G_OPTION_FLAG_REVERSE; } if (group->entries[i].arg != G_OPTION_ARG_CALLBACK && (group->entries[i].flags & (G_OPTION_FLAG_NO_ARG | G_OPTION_FLAG_OPTIONAL_ARG | G_OPTION_FLAG_FILENAME))) { g_warning(.....); group->entries[i].flags &= ~(G_OPTION_FLAG_NO_ARG | G_OPTION_FLAG_OPTIONAL_ARG | G_OPTION_FLAG_FILENAME); }
Thanks for the suggestions. I've done both of these.
Matthias, in the patch for this bug, you added two new methods that don't actually get used: option-context.c:1061:1: warning: ‘callback_test_optional_9’ defined but not used [-Wunused-function] option-context.c:1095:1: warning: ‘callback_test_optional_10’ defined but not used [-Wunused-function] I'm not sure if the methods are unnecessary or if the test is accidentally broken...
I believe they may have been used at some point, but then that was dropped ? anyway, if they aren't used, there shouldn't be much harm in dropping them...
ah, ok, it looks like they were testing cases that you later decided to turn into g_warnings rather than GErrors. I removed them.
Since that change, I am receiving the following warning: (process:11473): GLib-WARNING **: goption.c:2168: ignoring no-arg, optional-arg or filename flags (8) on option of type 0 These are my GOptionEntry static GOptionEntry options[] = { {"version", '\0', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_NONE, NULL, N_("Print application version"), NULL }, {"quiet", 'q', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_NONE, NULL, N_("Be quiet"), NULL }, {"command", 'c', 0, G_OPTION_ARG_STRING, NULL, N_("Command name"), "{info, play, convert, encode}" }, {"input-file", 'i', 0, G_OPTION_ARG_FILENAME, NULL, N_("Input file name"), N_("<songfile>") }, {"output-file", 'o', 0, G_OPTION_ARG_FILENAME, NULL, N_("Output file name"), N_("<songfile>") }, {NULL} }; options[0].arg_data=&arg_version; options[1].arg_data=&arg_quiet; options[2].arg_data=&command; options[3].arg_data=&input_file_name; options[4].arg_data=&output_file_name; First, the warning is quite unhelpful, as 1.) instead of "no-arg, optional-arg or filename flags (8)" it could simply tell which flags it looked at 2.) there is no "option of type 0", there are options-entries with arg=0 Besides these nitpicks, I don't see whats the purpose of the warning. The api-docs don't say anything about G_OPTION_ARG_FILENAME requiring G_OPTION_ARG_CALLBACK and it also works like above since years.
For reference, the commit is 52ef73ac8c40ea2a8ca80cf96d52a836bcaf76c2
You have misunderstood the warning. It does not warn about your use of G_OPTION_ARG_FILENAME. It warns about your use of G_OPTION_FLAG_NO_ARG. The documentation of G_OPTION_FLAG_NO_ARG does say that it's relevant only in combination with G_OPTION_ARG_CALLBACK. Perhaps this proves that you are right in saying that the warning is unhelpful.
Kjell, thanks for your quick reply: The warning led me to check the file-name args which are in fact fine. I will make a patch to fix the warning and mention which option entry it is complaining.
commit 2161bf254f6bfdfe589f7750c30f129261a48b30 Author: Stefan Sauer <ensonic@users.sf.net> Date: Sun Feb 26 21:20:24 2012 +0100 goption: try to be helpful in goption args/flag checks When complaining about ill defined GOptionEntries include the name of the option group and entry in the warning.