Re: [RFC PATCHv3] repack: rewrite the shell script in C.

["René Scharfe" <l.s.r@web.de>]

> +static void remove_temporary_files() {
> +	DIR *dir;
> +	struct dirent *e;
> +	char *prefix, *path;
> +
> +	prefix = mkpathdup(".tmp-%d-pack", getpid());
> +	path = mkpathdup("%s/pack", get_object_directory());
> +
> +	dir = opendir(path);
> +	while ((e = readdir(dir)) != NULL) {
> +		if (!prefixcmp(e->d_name, prefix)) {
> +			struct strbuf fname = STRBUF_INIT;
> +			strbuf_addf(&fname, "%s/%s", path, e->d_name);
> +			unlink(strbuf_detach(&fname, NULL));

I'm not sure I like the memory allocation done here for each file to be
deleted, but it's probably not worth worrying about.

> +void get_pack_sha1_list(char *packdir, struct string_list *sha1_list)
> +{
> +	DIR *dir;
> +	struct dirent *e;
> +	char *path, *suffix;
> +
> +	path = mkpathdup("%s/pack", get_object_directory());
> +	suffix = ".pack";
> +
> +	dir = opendir(path);
> +	while ((e = readdir(dir)) != NULL) {
> +		if (!suffixcmp(e->d_name, suffix)) {
> +			char *buf, *sha1;
> +			buf = xmemdupz(e->d_name, strlen(e->d_name));
> +			buf[strlen(e->d_name) - strlen(suffix)] = '\0';
> +			if (strlen(e->d_name) - strlen(suffix) > 40) {
> +				sha1 = &buf[strlen(e->d_name) - strlen(suffix) - 40];
> +				string_list_append_nodup(sha1_list, sha1);

Unless sha1 == buf, this will crash when that string_list is freed
because sha1 was not returned by malloc.  If it doesn't crash for
you then I guess sha1_list is never freed. :)  How about just
taking the part of d_name we need, like this?

			size_t len = strlen(e->d_name) - strlen(suffix);
			if (len > 40) {
				char *sha1 = xmemdupz(e->d_name + len - 40, 40);
				string_list_append_nodup(sha1_list, sha1);
			}

> +			} else {
> +				/*TODO: what should happen to pack files having no 40 char sha1 specifier?*/

What does the current code do with them?  From a quick glance it
looks like it deletes them in the end, right?

René