> That said, I'm not sure why you need split_cmdline() at all. The format
> seems to be:
>
> remote-object-info <url> <oid>...
>
> The only thing that _might_ need quoting is the url, but is shell
> quoting a reasonable thing there? I'd think that it would be
> URL-encoded, and thus contain no spaces. The <oid> has to be a real full
> oid, I think, because the object-info on the server side insists on
> that.
>
Thank you! We hadnâ??t given much thought to the URL format earlier,
but I agree that itâ??s reasonable to require the URL in
remote-object-info to be properly URL-encoded.
With that assumption, splitting on spaces makes sense. Iâ??ll update
this in the next patch and
also revise the documentation to clarify that URL parameters must be
URL-encoded.
> So why not just split on space? Something like this:
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 9de1016acd..aedbcba347 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -597,7 +597,7 @@ static void batch_one_object(const char *obj_name,
> object_context_release(&ctx);
> }
>
> -static int get_remote_info(struct batch_options *opt, int argc, const char **argv)
> +static int get_remote_info(struct batch_options *opt, const char *url, const char *oid_list)
> {
> int retval = 0;
> struct remote *remote = NULL;
> @@ -613,16 +613,19 @@ static int get_remote_info(struct batch_options *opt, int argc, const char **arg
> if (!opt->format)
> opt->format = "%(objectname) %(objectsize)";
>
> - remote = remote_get(argv[0]);
> + remote = remote_get(url);
> if (!remote)
> die(_("must supply valid remote when using remote-object-info"));
>
> oid_array_clear(&object_info_oids);
> - for (size_t i = 1; i < argc; i++) {
> - if (get_oid_hex(argv[i], &oid))
> - die(_("Not a valid object name %s"), argv[i]);
> + while (*oid_list) {
> + if (parse_oid_hex(oid_list, &oid, &oid_list))
> + die(_("Not a valid object name %s"), oid_list);
> oid_array_append(&object_info_oids, &oid);
> + while (*oid_list == ' ')
> + oid_list++;
> }
> +
> if (!object_info_oids.nr)
> die(_("remote-object-info requires objects"));
>
> @@ -747,21 +750,15 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
> const char *line, struct strbuf *output,
> struct expand_data *data)
> {
> - int count;
> - const char **argv;
> - char *line_to_split;
> -
> - if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
> - die(_("remote-object-info command input overflow "
> - "(no more than %d objects are allowed)"),
> - MAX_ALLOWED_OBJ_LIMIT);
> + char *url;
> + const char *space;
>
> - line_to_split = xstrdup(line);
> - count = split_cmdline(line_to_split, &argv);
> - if (count < 0)
> - die(_("split remote-object-info command"));
> + space = strchr(line, ' ');
> + if (!space)
> + return; /* report error somehow? */
> + url = xmemdupz(line, space - line);
>
> - if (get_remote_info(opt, count, argv))
> + if (get_remote_info(opt, url, space + 1))
> goto cleanup;
>
> data->skip_object_info = 1;
> @@ -774,16 +771,15 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
> */
> data->size = *remote_object_info[i].sizep;
> opt->batch_mode = BATCH_MODE_INFO;
> - batch_object_write(argv[i+1], output, opt, data, NULL, 0);
> + batch_object_write(oid_to_hex(&data->oid), output, opt, data, NULL, 0);
> }
> }
> data->skip_object_info = 0;
>
> cleanup:
> for (size_t i = 0; i < object_info_oids.nr; i++)
> free_object_info_contents(&remote_object_info[i]);
> - free(line_to_split);
> - free(argv);
> + free(url);
> free(remote_object_info);
> }
>
>
> You'd need to adjust t1017 to remote the quotes from the inputs, and I
> think you'd have to correctly url-encoded the file:// one to avoid
> spaces (but that is technically true already! If the filesystem path has
> a "%" in it, it would be misinterpreted).
>
Thank you. Tests will be adjusted.
> -Peff
