Re: [PATCH] last-modified: fix bug caused by inproper initialized memory

From: Jeff King <peff@peff.net>
Date: Dec 8, 2025, 20:15 UTC
Message-ID: <20251208201501.GA216526@coredump.intra.peff.net>
In-Reply-To: <87bjk9w5yv.fsf@iotcl.com>

On Mon, Dec 08, 2025 at 12:47:20PM +0100, Toon Claes wrote:

Show 10 quoted lines

> >     So it would probably have to be limited to something like:
> >
> >       CLEAR_ARRAY(lm->scratch->words, lm->scratch->word_alloc);
> >
> >     which I'd guess would cover most memset cases. But this is getting
> >     specific enough that maybe the macro is making things more confusing
> >     rather than less.
> 
> I've submitted a v2 that introduces MEMZERO_ARRAY(). I'm curious what
> the responses on this proposal are?

I think it looks fine, though as Junio noted, the original is already in next so it would have to be a patch on top.

Is such a macro worth it? I guess we'd be able to see if there are other possible sites with something like:

  git grep 'memset(.*0,.*\* \?sizeof'

that's looking for memsets of "0" that also multiply by sizeof. Looks like there are a few:

  add-patch.c:    memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk));
  builtin/last-modified.c:        memset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t));
  compat/simple-ipc/ipc-win32.c:  memset(ea, 0, NR_EA * sizeof(EXPLICIT_ACCESS));
  diff-delta.c:   memset(hash, 0, hsize * sizeof(*hash));
  hashmap.c:      memset(map->table, 0, map->tablesize * sizeof(struct hashmap_entry *));
  pack-revindex.c:                memset(pos, 0, BUCKETS * sizeof(*pos));

The first one is an oddball, but the other five could use it. So if we were to do a patch adding MEMZERO_ARRAY(), it would probably make sense to convert those spots. I'd be OK either way.

-Peff