* [PATCH 1/1] ubifs: do not decide upon uninitialized variable
@ 2020-12-25 14:19 Heinrich Schuchardt
2020-12-26 7:08 ` Chris Packham
2021-01-20 21:46 ` Tom Rini
0 siblings, 2 replies; 5+ messages in thread
From: Heinrich Schuchardt @ 2020-12-25 14:19 UTC (permalink / raw)
To: u-boot
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(-)
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/1] ubifs: do not decide upon uninitialized variable
2020-12-25 14:19 [PATCH 1/1] ubifs: do not decide upon uninitialized variable Heinrich Schuchardt
@ 2020-12-26 7:08 ` Chris Packham
2020-12-26 7:34 ` Heinrich Schuchardt
2021-01-20 21:46 ` Tom Rini
1 sibling, 1 reply; 5+ messages in thread
From: Chris Packham @ 2020-12-26 7:08 UTC (permalink / raw)
To: u-boot
(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
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] ubifs: do not decide upon uninitialized variable
2020-12-26 7:08 ` Chris Packham
@ 2020-12-26 7:34 ` Heinrich Schuchardt
2020-12-27 7:38 ` Chris Packham
0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2020-12-26 7:34 UTC (permalink / raw)
To: u-boot
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
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] ubifs: do not decide upon uninitialized variable
2020-12-26 7:34 ` Heinrich Schuchardt
@ 2020-12-27 7:38 ` Chris Packham
0 siblings, 0 replies; 5+ messages in thread
From: Chris Packham @ 2020-12-27 7:38 UTC (permalink / raw)
To: u-boot
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 at 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
> >>
> >>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] ubifs: do not decide upon uninitialized variable
2020-12-25 14:19 [PATCH 1/1] ubifs: do not decide upon uninitialized variable Heinrich Schuchardt
2020-12-26 7:08 ` Chris Packham
@ 2021-01-20 21:46 ` Tom Rini
1 sibling, 0 replies; 5+ messages in thread
From: Tom Rini @ 2021-01-20 21:46 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210120/f67d0eed/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-20 21:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-25 14:19 [PATCH 1/1] ubifs: do not decide upon uninitialized variable Heinrich Schuchardt
2020-12-26 7:08 ` Chris Packham
2020-12-26 7:34 ` Heinrich Schuchardt
2020-12-27 7:38 ` Chris Packham
2021-01-20 21:46 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.