Subject: Re: [PATCH 5/6] pack-bitmap-write.c: avoid uninitialized 'write_as' field

From: Taylor Blau <[email-masked]>
Date: Wed, 15 May 2024 09:29:14 -0400

On Wed, May 15, 2024 at 11:05:10AM +0200, Patrick Steinhardt wrote:
> > diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
> > index c0087dab12..420f17c2e0 100644
> > --- a/pack-bitmap-write.c
> > +++ b/pack-bitmap-write.c
> > @@ -112,6 +112,7 @@ static inline void push_bitmapped_commit(struct bitmap_writer *writer,
> >
> >  	writer->selected[writer->selected_nr].commit = commit;
> >  	writer->selected[writer->selected_nr].bitmap = NULL;
> > +	writer->selected[writer->selected_nr].write_as = NULL;
> >  	writer->selected[writer->selected_nr].flags = 0;
>
> Instead of having to ensure that all fields are initialized we could
> also set the whole structure to zero via `memset()`, which might be a
> bit more sustainable in the future. That alone doesn't really warrant a
> reroll though.

I had considered it, but decided against it as it seemed wasteful to

    memset(&writer->selected[writer->selected_nr], 0,
           sizeof(struct bitmapped_commit))

and then immediately un-zero some of its memory by assigning the commit
field.

Obviously not actually all that wasteful, as we're only talking about a
couple of hundred CPU cycles ;-).

Thanks,
Taylor