Re: [RFC PATCHv3] repack: rewrite the shell script in C.
- From
René Scharfe <l.s.r@web.de>
- Date
- Aug 18, 2013, 16:44 UTC
- Message-ID
- <5210F9FE.1080801@web.de>
- In-Reply-To
- <1376836587-25248-1-git-send-email-stefanbeller@googlemail.com>
Show 14 quoted lines
> +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.
Show 18 quoted lines
> +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é