Re: [PATCH] fix pack.packSizeLimit and --max-pack-size handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nicolas Pitre <nico@xxxxxxx> writes:

> @@ -2103,11 +2107,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>  			continue;
>  		}
>  		if (!prefixcmp(arg, "--max-pack-size=")) {
> -			char *end;
> -			pack_size_limit_cfg = 0;
> -			pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;
> -			if (!arg[16] || *end)
> +			if (!git_parse_ulong(arg+16, &pack_size_limit))
>  				usage(pack_usage);
> +			else
> +				pack_size_limit_cfg = 0;
>  			continue;
>  		}
>  		if (!prefixcmp(arg, "--window=")) {

Hmm, an unrelated funniness here is why the code even needs to futz with
the value read from the configuration.  Later in the code we have:

	if (!pack_to_stdout && !pack_size_limit)
		pack_size_limit = pack_size_limit_cfg;

and the intent seems to be:

 - if there is nothing on the command line, use config;
 - if there is something on the command line, ignore config.

But if you have a configured limit and would want to override from the
command line, this won't work.

I will apply the wraparound avoidance as a separate "pure fix" patch to
'maint' first.

Besides, --max-pack-size has been defined as megabytes for the entirety of
its existence, and I am a bit worried about changing the semantics at this
point without any warning.  I realize that I am worried too much for
people with a script that give an explicit --max-pack-size=1 to obtain a
set of split packs that would fit on floppies, who probably would not
exist ;-)
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux