| Message ID | 20201225141939.35426-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Commit | 81f562719e475b0e13ff7a0201cae19f8cc367fe |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/1] ubifs: do not decide upon uninitialized variable | expand |
(note replying on mobile device only code access is via git.denx.de) On Sat, 26 Dec 2020, 3:19 AM Heinrich Schuchardt, <xypron.glpk@gmx.de> wrote: > Before 'if (err)' we have to initialize the variable otherwise we use a > random value from the stack. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > In all the cases below err might be uninitialised because the code that would otherwise initialise it has been conditionally excluded with #ifndef __UBOOT__. I wonder if we could do something cleaner by making dbg_leb_change() etc do something suitable (return 0 or -Esomething). fs/ubifs/io.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c > index eb14b89544..9962cbe7eb 100644 > --- a/fs/ubifs/io.c > +++ b/fs/ubifs/io.c > @@ -114,7 +114,7 @@ int ubifs_leb_read(const struct ubifs_info *c, int > lnum, void *buf, int offs, > int ubifs_leb_write(struct ubifs_info *c, int lnum, const void *buf, int > offs, > int len) > { > - int err; > + int err = 0; > > ubifs_assert(!c->ro_media && !c->ro_mount); > if (c->ro_error) > @@ -136,7 +136,7 @@ int ubifs_leb_write(struct ubifs_info *c, int lnum, > const void *buf, int offs, > > int ubifs_leb_change(struct ubifs_info *c, int lnum, const void *buf, int > len) > { > - int err; > + int err = 0; > > ubifs_assert(!c->ro_media && !c->ro_mount); > if (c->ro_error) > @@ -158,7 +158,7 @@ int ubifs_leb_change(struct ubifs_info *c, int lnum, > const void *buf, int len) > > int ubifs_leb_unmap(struct ubifs_info *c, int lnum) > { > - int err; > + int err = 0; > > ubifs_assert(!c->ro_media && !c->ro_mount); > if (c->ro_error) > @@ -179,7 +179,7 @@ int ubifs_leb_unmap(struct ubifs_info *c, int lnum) > > int ubifs_leb_map(struct ubifs_info *c, int lnum) > { > - int err; > + int err = 0; > > ubifs_assert(!c->ro_media && !c->ro_mount); > if (c->ro_error) > -- > 2.29.2 > >
Am 26. Dezember 2020 08:08:52 MEZ schrieb Chris Packham <judge.packham@gmail.com>: >(note replying on mobile device only code access is via git.denx.de) > >On Sat, 26 Dec 2020, 3:19 AM Heinrich Schuchardt, <xypron.glpk@gmx.de> >wrote: > >> Before 'if (err)' we have to initialize the variable otherwise we use >a >> random value from the stack. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> > >In all the cases below err might be uninitialised because the code that >would otherwise initialise it has been conditionally excluded with >#ifndef >__UBOOT__. I wonder if we could do something cleaner by making >dbg_leb_change() etc do something suitable (return 0 or -Esomething). Why would that be cleaner? Best regards Heinrich > > fs/ubifs/io.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c >> index eb14b89544..9962cbe7eb 100644 >> --- a/fs/ubifs/io.c >> +++ b/fs/ubifs/io.c >> @@ -114,7 +114,7 @@ int ubifs_leb_read(const struct ubifs_info *c, >int >> lnum, void *buf, int offs, >> int ubifs_leb_write(struct ubifs_info *c, int lnum, const void *buf, >int >> offs, >> int len) >> { >> - int err; >> + int err = 0; >> >> ubifs_assert(!c->ro_media && !c->ro_mount); >> if (c->ro_error) >> @@ -136,7 +136,7 @@ int ubifs_leb_write(struct ubifs_info *c, int >lnum, >> const void *buf, int offs, >> >> int ubifs_leb_change(struct ubifs_info *c, int lnum, const void >*buf, int >> len) >> { >> - int err; >> + int err = 0; >> >> ubifs_assert(!c->ro_media && !c->ro_mount); >> if (c->ro_error) >> @@ -158,7 +158,7 @@ int ubifs_leb_change(struct ubifs_info *c, int >lnum, >> const void *buf, int len) >> >> int ubifs_leb_unmap(struct ubifs_info *c, int lnum) >> { >> - int err; >> + int err = 0; >> >> ubifs_assert(!c->ro_media && !c->ro_mount); >> if (c->ro_error) >> @@ -179,7 +179,7 @@ int ubifs_leb_unmap(struct ubifs_info *c, int >lnum) >> >> int ubifs_leb_map(struct ubifs_info *c, int lnum) >> { >> - int err; >> + int err = 0; >> >> ubifs_assert(!c->ro_media && !c->ro_mount); >> if (c->ro_error) >> -- >> 2.29.2 >> >>
On Sat, 26 Dec 2020, 8:34 PM Heinrich Schuchardt, <xypron.glpk@gmx.de> wrote: > Am 26. Dezember 2020 08:08:52 MEZ schrieb Chris Packham < > judge.packham@gmail.com>: > >(note replying on mobile device only code access is via git.denx.de) > > > >On Sat, 26 Dec 2020, 3:19 AM Heinrich Schuchardt, <xypron.glpk@gmx.de> > >wrote: > > > >> Before 'if (err)' we have to initialize the variable otherwise we use > >a > >> random value from the stack. > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> --- > >> > > > >In all the cases below err might be uninitialised because the code that > >would otherwise initialise it has been conditionally excluded with > >#ifndef > >__UBOOT__. I wonder if we could do something cleaner by making > >dbg_leb_change() etc do something suitable (return 0 or -Esomething). > > Why would that be cleaner? > It would keep the deviations to the upstream UBIFS code to a minimum. It's not too hard to ignore the '= 0' when syncing with upstream but it'd also be easy to miss and we'd have to redo this patch. Of course this is all hypothetical and my opinion carries little weight so I'm more than happy to be ignored if Tom wants to apply this as is. > > Best regards > > Heinrich > > > > > fs/ubifs/io.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c > >> index eb14b89544..9962cbe7eb 100644 > >> --- a/fs/ubifs/io.c > >> +++ b/fs/ubifs/io.c > >> @@ -114,7 +114,7 @@ int ubifs_leb_read(const struct ubifs_info *c, > >int > >> lnum, void *buf, int offs, > >> int ubifs_leb_write(struct ubifs_info *c, int lnum, const void *buf, > >int > >> offs, > >> int len) > >> { > >> - int err; > >> + int err = 0; > >> > >> ubifs_assert(!c->ro_media && !c->ro_mount); > >> if (c->ro_error) > >> @@ -136,7 +136,7 @@ int ubifs_leb_write(struct ubifs_info *c, int > >lnum, > >> const void *buf, int offs, > >> > >> int ubifs_leb_change(struct ubifs_info *c, int lnum, const void > >*buf, int > >> len) > >> { > >> - int err; > >> + int err = 0; > >> > >> ubifs_assert(!c->ro_media && !c->ro_mount); > >> if (c->ro_error) > >> @@ -158,7 +158,7 @@ int ubifs_leb_change(struct ubifs_info *c, int > >lnum, > >> const void *buf, int len) > >> > >> int ubifs_leb_unmap(struct ubifs_info *c, int lnum) > >> { > >> - int err; > >> + int err = 0; > >> > >> ubifs_assert(!c->ro_media && !c->ro_mount); > >> if (c->ro_error) > >> @@ -179,7 +179,7 @@ int ubifs_leb_unmap(struct ubifs_info *c, int > >lnum) > >> > >> int ubifs_leb_map(struct ubifs_info *c, int lnum) > >> { > >> - int err; > >> + int err = 0; > >> > >> ubifs_assert(!c->ro_media && !c->ro_mount); > >> if (c->ro_error) > >> -- > >> 2.29.2 > >> > >> > >
On Fri, Dec 25, 2020 at 03:19:39PM +0100, Heinrich Schuchardt wrote: > Before 'if (err)' we have to initialize the variable otherwise we use a > random value from the stack. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Applied to u-boot/master, thanks!
diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c index eb14b89544..9962cbe7eb 100644 --- a/fs/ubifs/io.c +++ b/fs/ubifs/io.c @@ -114,7 +114,7 @@ int ubifs_leb_read(const struct ubifs_info *c, int lnum, void *buf, int offs, int ubifs_leb_write(struct ubifs_info *c, int lnum, const void *buf, int offs, int len) { - int err; + int err = 0; ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) @@ -136,7 +136,7 @@ int ubifs_leb_write(struct ubifs_info *c, int lnum, const void *buf, int offs, int ubifs_leb_change(struct ubifs_info *c, int lnum, const void *buf, int len) { - int err; + int err = 0; ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) @@ -158,7 +158,7 @@ int ubifs_leb_change(struct ubifs_info *c, int lnum, const void *buf, int len) int ubifs_leb_unmap(struct ubifs_info *c, int lnum) { - int err; + int err = 0; ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error) @@ -179,7 +179,7 @@ int ubifs_leb_unmap(struct ubifs_info *c, int lnum) int ubifs_leb_map(struct ubifs_info *c, int lnum) { - int err; + int err = 0; ubifs_assert(!c->ro_media && !c->ro_mount); if (c->ro_error)
Before 'if (err)' we have to initialize the variable otherwise we use a random value from the stack. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- fs/ubifs/io.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) -- 2.29.2