Re: [PATCH v4 03/16] index-helper: new daemon for caching index and related stuff

[David Turner <dturner@twopensource.com>]

On Fri, 2016-04-15 at 17:04 -0700, Stefan Beller wrote:
> > +static int try_shm(struct index_state *istate)
> > +{
> > +       void *new_mmap = NULL;
> > +       size_t old_size = istate->mmap_size;
> > +       ssize_t new_size;
> > +       const unsigned char *sha1;
> > +       struct stat st;
> > +       int fd;
> > +
> > +       if (!is_main_index(istate) ||
> > +           old_size <= 20 ||
> > +           stat(git_path("index-helper.path"), &st))
> > +               return -1;
> > +       if (poke_daemon(istate, &st, 0))
> > +               return -1;
> > +       sha1 = (unsigned char *)istate->mmap + old_size - 20;
> > +
> > +       fd = open(index_helper_path("git-index-%s",
> > sha1_to_hex(sha1)),
> > +                 O_RDONLY);
> > +       if (fd < 0)
> > +               goto fail;
> > +
> > +       if (fstat(fd, &st))
> > +               goto fail;
> > +
> > +       new_size = st.st_size;
> > +       new_mmap = mmap(NULL, new_size, PROT_READ, MAP_SHARED, fd,
> > 0);
> > +       if (new_size <= 20 ||
> > +           hashcmp((unsigned char *)istate->mmap + old_size - 20,
> > +                   (unsigned char *)new_mmap + new_size - 20)) {
> > +               if (new_mmap)
> > +                       munmap(new_mmap, new_size);
> > +               goto fail;
> 
> coming from here
> 
> > +       }
> > +
> > +       /* The memory barrier here matches index
> > -helper.c:share_index. */
> > +       __sync_synchronize();
> > +
> > +       munmap(istate->mmap, istate->mmap_size);
> > +       istate->mmap = new_mmap;
> > +       istate->mmap_size = new_size;
> > +       istate->from_shm = 1;
> > +       return 0;
> > +
> > +fail:
> 
> fd may be leaking here?
> 
> > +       poke_daemon(istate, &st, 1);
> > +       return -1;
> > +}
> > +


Good point.  (It's also leaking on the happy path -- will fix all of
those)