Eric Ju <eric.peijian@xxxxxxxxx> writes:
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 69ea642dc6..47fd2a777b 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -27,6 +27,18 @@
> #include "promisor-remote.h"
> #include "mailmap.h"
> #include "write-or-die.h"
> +#include "alias.h"
> +#include "remote.h"
> +#include "transport.h"
> +
> +/* Maximum length for a remote URL. While no universal standard exists,
> + * 8K is assumed to be a reasonable limit.
> + */
Style. Our multi-line comment begins with slash-asterisk and ends
with asterisk-slash both on their own line without anything else.
> +#define MAX_REMOTE_URL_LEN (8*1024)
Here and ...
> +/* Maximum number of objects allowed in a single remote-object-info request. */
> +#define MAX_ALLOWED_OBJ_LIMIT 10000
... here, please have a blank line.
> +/* Maximum input size permitted for the remote-object-info command. */
> +#define MAX_REMOTE_OBJ_INFO_LINE (MAX_REMOTE_URL_LEN + MAX_ALLOWED_OBJ_LIMIT * (GIT_MAX_HEXSZ + 1))
This is an overly long line.
> @@ -579,6 +593,61 @@ 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)
> +{
> + int retval = 0;
> + struct remote *remote = NULL;
> + struct object_id oid;
> + struct string_list object_info_options = STRING_LIST_INIT_NODUP;
> + static struct transport *gtransport;
> +
> + /*
> + * Change the format to "%(objectname) %(objectsize)" when
> + * remote-object-info command is used. Once we start supporting objecttype
> + * the default format should change to DEFAULT_FORMAT.
> + */
Style. Closing asterisk-slash aligns with the asterisk on the
previous line.
> + if (!opt->format)
> + opt->format = "%(objectname) %(objectsize)";
> +
> + remote = remote_get(argv[0]);
> + 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++) {
Pointless mixing of "size_t" and "int". We have declared "int
argc", which is perfectly a sensible type, since we know that the
value of it would not exceed MAX_ALLOWED_OBJ_LIMIT, which is 10000.
> + if (get_oid_hex(argv[i], &oid))
> + die(_("Not a valid object name %s"), argv[i]);
> + oid_array_append(&object_info_oids, &oid);
> + }
> + if (!object_info_oids.nr)
> + die(_("remote-object-info requires objects"));
> +
> + gtransport = transport_get(remote, NULL);
> + if (gtransport->smart_options) {
> + CALLOC_ARRAY(remote_object_info, object_info_oids.nr);
> + gtransport->smart_options->object_info = 1;
> + gtransport->smart_options->object_info_oids = &object_info_oids;
> +
> + /* 'objectsize' is the only option currently supported */
> + if (!strstr(opt->format, "%(objectsize)"))
> + die(_("%s is currently not supported with remote-object-info"), opt->format);
> +
> + string_list_append(&object_info_options, "size");
> +
> + if (object_info_options.nr > 0) {
> + gtransport->smart_options->object_info_options = &object_info_options;
> + gtransport->smart_options->object_info_data = remote_object_info;
> + retval = transport_fetch_refs(gtransport, NULL);
> + }
> + } else {
> + retval = -1;
> + }
Minor style nit, but when everything else is equal, writing the side
of smaller body first would make it easier to follow if/else, i.e.
gtransport = transport_get(remote, NULL);
if (!gtransport->smart_options) {
/* error */
retval = -1;
} else {
... a lot of real code here ...
}
> +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);
Nobody guarantees this user gave a request for more than 10000
objects; after all it may have been an overly long URL that busted
the line length limit, no?
> + line_to_split = xstrdup(line);
> + count = split_cmdline(line_to_split, &argv);
> + if (count < 0)
> + die(_("split remote-object-info command"));
Here, the code could check if count busts MAX_ALLOWED_OBJ_LIMIT, but
it doesn't.
