On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
<christian.couder@xxxxxxxxx> wrote:
> +static int can_delete_shared_index(const char *shared_sha1_hex)
> +{
> + struct stat st;
> + unsigned long expiration;
> + const char *shared_index = git_path("sharedindex.%s", shared_sha1_hex);
> +
> + /* Check timestamp */
> + expiration = get_shared_index_expire_date();
> + if (!expiration)
> + return 0;
> + if (stat(shared_index, &st))
> + return error_errno("could not stat '%s", shared_index);
> + if (st.st_mtime > expiration)
I wonder if we should check ctime too, in case mtime is not reliable
(and ctime is less likely to be manipulated by user), just for extra
safety. If (st.st_mtime > expiration || st.st_ctime > expiration).
> + return 0;
> +
> + return 1;
> +}
> +
> +static void clean_shared_index_files(const char *current_hex)
> +{
> + struct dirent *de;
> + DIR *dir = opendir(get_git_dir());
> +
> + if (!dir) {
> + error_errno("unable to open git dir: %s", get_git_dir());
_()
> + return;
Or just do "return error_errno(...)". The caller can ignore the return
value for now.
> + }
> +
> + while ((de = readdir(dir)) != NULL) {
> + const char *sha1_hex;
> + if (!skip_prefix(de->d_name, "sharedindex.", &sha1_hex))
> + continue;
> + if (!strcmp(sha1_hex, current_hex))
> + continue;
Yeah.. make sure that the shared index linked to $GIT_DIR/index stay,
even if mtime is screwed up. I wonder if we should have the same
treatment for $GIT_DIR/index.lock though, as an extra safety measure.
If you call this function in write_locked_index, then
$GIT_DIR/index.lock is definitely there. Hmm.. maybe it _is_
current_hex, current_hex is not the value from $GIT_DIR/index...
> + if (can_delete_shared_index(sha1_hex) > 0 &&
> + unlink(git_path("%s", de->d_name)))
> + error_errno("unable to unlink: %s", git_path("%s", de->d_name));
_()
> static int write_shared_index(struct index_state *istate,
> @@ -2211,8 +2269,11 @@ static int write_shared_index(struct index_state *istate,
> }
> ret = rename_tempfile(&temporary_sharedindex,
> git_path("sharedindex.%s", sha1_to_hex(si->base->sha1)));
> - if (!ret)
> + if (!ret) {
> hashcpy(si->base_sha1, si->base->sha1);
> + clean_shared_index_files(sha1_to_hex(si->base->sha1));
This operation is technically garbage collection and should belong to
"git gc --auto", which is already called automatically in a few
places. Is it not called often enough that we need to do the cleaning
up right after a new shared index is created?
--
Duy