Derrick Stolee <stolee@xxxxxxxxx> writes: > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > Date: Fri, 24 Apr 2020 13:11:13 +0000 > Subject: [PATCH] multi-pack-index: close file descriptor after mmap > > We recently discovered that the commit-graph was not closing its > file descriptor after memory-mapping the file contents. After this > mmap() succeeds, there is no need to keep the file descriptor open. > In fact, there is signficant reason to close it so we do not run > out of descriptors. The above is sufficient a justification. Let's leave the remaining two paragraphs under three-dashes. > This is entirely my fault for not knowing that we can have an open > mmap while also closing the descriptor. Some could blame this on > being a new contributor when the series was first submitted, but > even years later this is still new information to me. > > That made me realize that I used the same pattern when opening a > multi-pack-index. Since this file is not (yet) incremental, there > will be at most one descriptor open for this reason. It is still > worth fixing, especially if we extend the format to be incremental > like the commit-graph. > > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > midx.c | 4 +--- > midx.h | 2 -- > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/midx.c b/midx.c > index 1527e464a7..60d30e873b 100644 > --- a/midx.c > +++ b/midx.c > @@ -72,9 +72,9 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local > FREE_AND_NULL(midx_name); > > midx_map = xmmap(NULL, midx_size, PROT_READ, MAP_PRIVATE, fd, 0); > + close(fd); > > FLEX_ALLOC_STR(m, object_dir, object_dir); > - m->fd = fd; > m->data = midx_map; > m->data_len = midx_size; > m->local = local; > @@ -190,8 +190,6 @@ void close_midx(struct multi_pack_index *m) > return; > > munmap((unsigned char *)m->data, m->data_len); > - close(m->fd); > - m->fd = -1; > > for (i = 0; i < m->num_packs; i++) { > if (m->packs[i]) > diff --git a/midx.h b/midx.h > index e6fa356b5c..b18cf53bc4 100644 > --- a/midx.h > +++ b/midx.h > @@ -12,8 +12,6 @@ struct repository; > struct multi_pack_index { > struct multi_pack_index *next; > > - int fd; > - > const unsigned char *data; > size_t data_len;