[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/6] loader/i386/bsd: Initialize ptr variable in grub_bsd_add
From: |
Daniel Kiper |
Subject: |
Re: [PATCH 2/6] loader/i386/bsd: Initialize ptr variable in grub_bsd_add_meta() |
Date: |
Thu, 24 Mar 2022 16:12:31 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Fri, Mar 18, 2022 at 02:21:23PM -0500, Glenn Washburn wrote:
> On Fri, 18 Mar 2022 08:57:27 +0100
> Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> > Dear Daniel,
> >
> >
> > Am 11.03.22 um 00:35 schrieb Daniel Kiper:
> > > Latest GCC may complain in that way:
> >
> > Just for the record, what is the latest GCC for you? 12?
>
> I reported this issue on Debian 11 GCC version 10.2.1[1].
Yeah, I could be more precise here.
> > > In file included from ../include/grub/disk.h:31,
> > > from ../include/grub/file.h:26,
> > > from ../include/grub/loader.h:23,
> > > from loader/i386/bsd.c:19:
> > > loader/i386/bsd.c: In function ‘grub_cmd_openbsd’:
> > > ../include/grub/misc.h:71:10: error: ‘ptr’ may be used uninitialized
> > > in this function [-Werror=maybe-uninitialized]
> > > 71 | return grub_memmove (dest, src, n);
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > loader/i386/bsd.c:266:9: note: ‘ptr’ was declared here
> > > 266 | void *ptr;
> > > | ^~~
> > >
> > > So, let's fix it by assigning NULL to ptr in grub_bsd_add_meta().
> > >
> > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> > > ---
> > > grub-core/loader/i386/bsd.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
> > > index 5f3290ce1..346c4f14a 100644
> > > --- a/grub-core/loader/i386/bsd.c
> > > +++ b/grub-core/loader/i386/bsd.c
> > > @@ -263,7 +263,7 @@ grub_err_t
> > > grub_bsd_add_meta (grub_uint32_t type, const void *data, grub_uint32_t
> > > len)
> > > {
> > > grub_err_t err;
> > > - void *ptr;
> > > + void *ptr = NULL;
> > >
> > > err = grub_bsd_add_meta_ptr (type, &ptr, len);
> > > if (err)
> >
> > It’s a bogus warning though, as `grub_bsd_add_meta_ptr()` return an
> > error when ptr wasn’t initialized yet, right?
>
> Yes, as far as I can tell this is bogus and noted that this seems to be
> a GCC issue[1]. I was only able to trigger this when using -O2, which
> seems odd to me.
<handwaving>
I think compiler is not able to infer any sensible relation between
grub_errno value and value returned from grub_malloc() because here
compiler sees only grub_malloc() prototype. So, that is why it complains
here. Of course we are smarter because we know how grub_malloc() works.
Or at least we think we know how it works... :-)
</handwaving>
Anyway, I think it is the simplest fix for this issue. Additionally, it
puts us on safe side if we wrongly assumed correct grub_malloc() behavior
in all cases...
Daniel