On Mon, Aug 03, 2015 at 19:38:15 -0400, Eric Sunshine wrote:
> On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel <mathstuf@xxxxxxxxx> wrote:
> > + OPT_BOOL('\0', "push", &push_mode,
> > + N_("query push URLs")),
>
> A bit more explanatory:
>
> "query push URLs rather than fetch URLs"
Fixed.
> > + OPT_BOOL('\0', "all", &all_mode,
> > + N_("return all URLs")),
> > + OPT_END()
> > + };
> > + argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage,
> > + PARSE_OPT_KEEP_ARGV0);
>
> What is the reason for PARSE_OPT_KEEP_ARGV0 in this case?
Copied from get-url; I presume for more natural argv[] usage within the
function.
> > + if (argc < 1 || argc > 2)
> > + usage_with_options(builtin_remote_geturl_usage, options);
>
> So, 'argc' must be 1 or 2, which in 'argv' terms is argv[0] and argv[1]).
…
> > + remotename = argv[1];
>
> But here, argv[1] is accessed unconditionally, even though 'argc' may
> have been 1, thus out of bounds.
Yep, should be (argc < 2 || argc > 2) (or 1 if PARSE_OPT_KEEP_ARGV0 is
removed). Off-by-one when converting from get-url.
I'll reroll tomorrow morning in case there are more comments until then
(particularly about PARSE_OPT_KEEP_ARGV0).
Thanks,
--Ben
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html