Re: [PATCH] Make git prune remove temporary packs that look like write failures

From: Brandon Casey <casey@nrlssc.navy.mil>
Date: Feb 6, 2008, 19:41 UTC
Message-ID: <47AA0D60.60504@nrlssc.navy.mil>
In-Reply-To: <e1dab3980802061059m5bf9c291s892da586248e229c@mail.gmail.com>

David Tweed wrote:

Show 13 quoted lines

> On Feb 6, 2008 4:48 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> I also suggest taking a look at the functions in builtin-prune-packed.c to see
>> how similar functions are implemented there.
>>
>> Use strlcpy instead of sprintf.
>> Use prefixcmp instead of strncmp.
>> Use same messages as prune_dir() for show_only path and unlink failure.
>> You could also check the opendir call and print a suitable message on failure.
> 
> All the other path creation in builtin-prune.c is using sprintf; is
> doing 3 strlcpy's much better? (I backed off from using snprintf when
> the other element in the if that tested it vanished; I probably ought
> to put that back.)

They use sprintf for the "%02x" part, but they use memcpy to copy the return of get_object_directory() into a fixed string and then append onto that, rather than repeatedly writing the same string over and over. Ok, there is one instance in builtin-prune.c that repeatedly writes path, but builtin-prune-packed.c does the memcpy thing.

Something like:

	char pathname[PATH_MAX];
	...

	/* check length of dirname not too long */

	memcpy(pathname, dirname, len);

	if (len && pathname[len-1] != '/')
		pathname[len++] = '/';

...

	while ((de = readdir(dir)...) {
		if (!prefixcmp(...
			if (strlcpy(pathname + len, de->d_name, PATH_MAX - len)
			    >= PATH_MAX - len) {
				warning("too long path encountered: %s%s",
					pathname, de->d_name);
				continue;
			}

...

-brandon