| Message ID | 20200321114615.5360-2-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | misc: Trivial static code analyzer fixes | expand |
12:49 PM Sub, 21.03.2020. Philippe Mathieu-Daudé <philmd@redhat.com> је написао/ла: > > Fix warning reported by Clang static code analyzer: > > block.c:3167:5: warning: Value stored to 'ret' is never read > ret = bdrv_fill_options(&options, filename, &flags, &local_err); > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Reported-by: Clang Static Analyzer Peter, and others, Is this allowed use of "Reported-by:" mark? I did not notice it being used this way before. And was under impression that all similar tags/marks must be followed by a person, not a tool. Regards, Aleksandar > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index a2542c977b..908c109a8c 100644 > --- a/block.c > +++ b/block.c > @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, > parent->open_flags, parent->options); > } > > - ret = bdrv_fill_options(&options, filename, &flags, &local_err); > + bdrv_fill_options(&options, filename, &flags, &local_err); > if (local_err) { > goto fail; > } > -- > 2.21.1 > >
Le 21/03/2020 à 12:46, Philippe Mathieu-Daudé a écrit : > Fix warning reported by Clang static code analyzer: > > block.c:3167:5: warning: Value stored to 'ret' is never read > ret = bdrv_fill_options(&options, filename, &flags, &local_err); > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Reported-by: Clang Static Analyzer > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index a2542c977b..908c109a8c 100644 > --- a/block.c > +++ b/block.c > @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, > parent->open_flags, parent->options); > } > > - ret = bdrv_fill_options(&options, filename, &flags, &local_err); > + bdrv_fill_options(&options, filename, &flags, &local_err); > if (local_err) { > goto fail; > } > I would be sruprised if coverity doesn't warn about an unused return value. Thanks, Laurent
Laurent Vivier <laurent@vivier.eu> writes: > Le 21/03/2020 à 12:46, Philippe Mathieu-Daudé a écrit : >> Fix warning reported by Clang static code analyzer: >> >> block.c:3167:5: warning: Value stored to 'ret' is never read >> ret = bdrv_fill_options(&options, filename, &flags, &local_err); >> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> Reported-by: Clang Static Analyzer >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> block.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/block.c b/block.c >> index a2542c977b..908c109a8c 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, >> parent->open_flags, parent->options); >> } >> >> - ret = bdrv_fill_options(&options, filename, &flags, &local_err); >> + bdrv_fill_options(&options, filename, &flags, &local_err); >> if (local_err) { >> goto fail; >> } >> > > I would be sruprised if coverity doesn't warn about an unused return value. Coverity recognizes the fact that some return values can be safely ignored, and reports only ignored return values it sees commonly checked elsewhere. This function is used called nowhere else. Coverity won't complain. However, I'd prefer ret = bdrv_fill_options(&options, filename, &flags, &local_err); - if (local_err) { + if (ret < 0) { goto fail; }
On Sat, 21 Mar 2020, Aleksandar Markovic wrote: > 12:49 PM Sub, 21.03.2020. Philippe Mathieu-Daudé <philmd@redhat.com> је > написао/ла: >> >> Fix warning reported by Clang static code analyzer: >> >> block.c:3167:5: warning: Value stored to 'ret' is never read >> ret = bdrv_fill_options(&options, filename, &flags, &local_err); >> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> Reported-by: Clang Static Analyzer > > Peter, and others, > > Is this allowed use of "Reported-by:" mark? > > I did not notice it being used this way before. And was under impression > that all similar tags/marks must be followed by a person, not a tool. I think there were previous patches with Reported-by: Euler Robot which may be similar to this usage. Regards, BALATON Zoltan > > Regards, > Aleksandar > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> block.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/block.c b/block.c >> index a2542c977b..908c109a8c 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const > char *filename, >> parent->open_flags, parent->options); >> } >> >> - ret = bdrv_fill_options(&options, filename, &flags, &local_err); >> + bdrv_fill_options(&options, filename, &flags, &local_err); >> if (local_err) { >> goto fail; >> } >> -- >> 2.21.1 >> >> >
diff --git a/block.c b/block.c index a2542c977b..908c109a8c 100644 --- a/block.c +++ b/block.c @@ -3164,7 +3164,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, parent->open_flags, parent->options); } - ret = bdrv_fill_options(&options, filename, &flags, &local_err); + bdrv_fill_options(&options, filename, &flags, &local_err); if (local_err) { goto fail; }
Fix warning reported by Clang static code analyzer: block.c:3167:5: warning: Value stored to 'ret' is never read ret = bdrv_fill_options(&options, filename, &flags, &local_err); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)