On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
[off-list ref] 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));

_()

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