On Thu, Jun 06, 2024 at 07:15:38AM +0200, Patrick Steinhardt wrote:
> 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
I've sent out a patch series [1] that would have made CI detect this
issue before hitting any of the mainline branches.
[1]: https://lore.kernel.org/git/cover.1717655210.git.ps@xxxxxx/
Patrick
Attachment:
signature.asc
Description: PGP signature
