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

> On Thu, Jun 06, 2024 at 06:50:56AM +0200, Patrick Steinhardt wrote:
> > 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