On Thu, 2 Aug 2018 15:20:33 -0400 Jeff King <peff@xxxxxxxx> wrote: > On Thu, Aug 02, 2018 at 11:47:30AM -0700, Stefan Beller wrote: > > > > +static int module_config(int argc, const char **argv, const char *prefix) > > > +{ > > > + if (argc < 2 || argc > 3) > > > + die("submodule--helper config takes 1 or 2 arguments: name [value]"); > > > + > > > + /* Equivalent to ACTION_GET in builtin/config.c */ > > > + if (argc == 2) > > > + return print_config_from_gitmodules(argv[1]); > > > + > > > + /* Equivalent to ACTION_SET in builtin/config.c */ > > > + if (argc == 3) > > > + return config_set_in_gitmodules_file_gently(argv[1], argv[2]); > > > + > > > + return 0; > > > > Technically we cannot reach this point here? > > Maybe it would be more defensive to > > > > BUG("How did we get here?"); > > > > or at least return something !=0 ? > > When I find myself reaching for a BUG(), it is often a good time to see > if we can restructure the code so that the logic more naturally falls > out. Here the issue is that the first if conditional repeats the "else" > for the other two. So I think we could just write: > > if (argc == 2) > return ... > if (argc == 3) > return ... > > die("need 1 or 2 arguments"); > Hi Jeff, I like that, I'll see how this plays out after patch 06 which adds another option, and decide whether to use this style; validating arguments beforehand may still look a little cleaner. Thanks for the comment. Ciao, Antonio -- Antonio Ospite https://ao2.it https://twitter.com/ao2it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?