Stefan Beller <stefanbel...@googlemail.com> writes:

> @@ -41,18 +35,16 @@ static void remove_temporary_files(void)
>       DIR *dir;
>       struct dirent *e;
>  
> +     dir = opendir(packdir);
> +     if (!dir)
>               return;
>  
> +     strbuf_addstr(&buf, packdir);
> +
> +     /* dirlen holds the length of the path before the file name */
>       dirlen = buf.len + 1;
> +     strbuf_addf(&buf, "%s", packtmp);
> +     /* prefixlen holds the length of the prefix */

Thanks to the name of the variable that is self-describing, this
comment does not add much value.

But it misses the whole point of my suggestion in the earlier
message to phrase these like so:

        /* Point at the slash at the end of ".../objects/pack/" */
        dirlen = strlen(packdir) + 1;
        /* Point at the dash at the end of ".../.tmp-%d-pack-" */
        prefixlen = buf.len - dirlen;

to clarify what the writer considers as "the prefix" is, which may
be quite different from what the readers think "the prefix" is.  In
".tmp-2342-pack-0d8beaa5b76e824c9869f0d1f1b19ec7acf4982f.pack", is
the prefix ".tmp-2342-", ".tmp-2342-pack", or ".tmp-2342-pack-"?

>  int cmd_repack(int argc, const char **argv, const char *prefix)
>  {
> ...
>       packdir = mkpathdup("%s/pack", get_object_directory());
>       packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
>  
> +     sigchain_push_common(remove_pack_on_signal);
> +
>       argv_array_push(&cmd_args, "pack-objects");
>       argv_array_push(&cmd_args, "--keep-true-parents");
>       argv_array_push(&cmd_args, "--honor-pack-keep");
> ...
> +                                     rollback_failure.items[i].string,
> +                                     rollback_failure.items[i].string);
>               }
>               exit(1);
>       }

The scripted version uses

    trap 'rm -f "$PACKTMP"-*' 0 1 2 3 15

so remove_temporary_files() needs to be called before exiting from
the program without getting killed by a signal.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to