Re: [PATCH v4 06/12] refs/files: extract function to iterate through root refs

From: Patrick Steinhardt <ps@pks.im>
Date: Jun 6, 2024, 05:15 UTC
Message-ID: <ZmFF-kgZSlB6O7Gs@tanuki>
In-Reply-To: <ZmFAMBhkuUQEhgCR@tanuki>

On Thu, Jun 06, 2024 at 06:50:56AM +0200, Patrick Steinhardt wrote:

Show 19 quoted lines

> On Wed, Jun 05, 2024 at 06:07:28AM -0400, Jeff King wrote:
> > On Mon, Jun 03, 2024 at 11:30:35AM +0200, Patrick Steinhardt wrote:
> > 
> > > +static int for_each_root_ref(struct files_ref_store *refs,
> > > +			     int (*cb)(const char *refname, void *cb_data),
> > > +			     void *cb_data)
> > >  {
> > >  	struct strbuf path = STRBUF_INIT, refname = STRBUF_INIT;
> > >  	const char *dirname = refs->loose->root->name;
> > >  	struct dirent *de;
> > >  	size_t dirnamelen;
> > > +	int ret;
> > >  	DIR *d;
> > 
> > Should we initialize ret to 0 here?
> 
> Yeah, we should. Or rather, I'll set `ret = 0;` on the successful path.
> 
> Patrick

I was wondering why the compiler didn't flag it, because I know that GCC has `-Wmaybe-uninitialized`. Turns out that this warning only works when having optimizations enabled, but if we do then it correctly flags this use:

     (git) ~/Development/git:HEAD $ make refs/files-backend.o
        * new build flags
        CC refs/files-backend.o
    refs/files-backend.c: In function ‘for_each_root_ref’:
    refs/files-backend.c:371:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
      371 |         return ret;
          |                ^~~
    refs/files-backend.c:334:13: note: ‘ret’ was declared here
      334 |         int ret;
          |             ^~~
    cc1: all warnings being treated as errors

I'll have a look at our CI jobs and adapt my own config.mak to include `-Og`.

Patrick