qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL
@ 2017-01-09 19:05 Peter Maydell
  2017-01-09 19:10 ` no-reply
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Peter Maydell @ 2017-01-09 19:05 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial; +Cc: patches, Edgar E. Iglesias

Coverity (CID 1005689) warns that we don't check that
spec_reg_info() returned non-NULL before dereferencing.
Add the check, though as the comment notes this is
a can't-really-happen case because the earlier constraint
matching should have ruled out the "unknown reg" case.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 disas/cris.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/cris.c b/disas/cris.c
index 08161d1..8a1daf9 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -2490,7 +2490,7 @@ print_with_operands (const struct cris_opcode *opcodep,
 	const struct cris_spec_reg *sregp
 	  = spec_reg_info ((insn >> 12) & 15, disdata->distype);
 
-	if (sregp->name == NULL)
+	if (sregp == NULL || sregp->name == NULL)
 	  /* Should have been caught as a non-match earlier.  */
 	  *tp++ = '?';
 	else
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL
  2017-01-09 19:05 [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL Peter Maydell
@ 2017-01-09 19:10 ` no-reply
  2017-01-09 21:35   ` Peter Maydell
  2017-01-09 20:13 ` Edgar E. Iglesias
  2017-01-12 11:03 ` Michael Tokarev
  2 siblings, 1 reply; 6+ messages in thread
From: no-reply @ 2017-01-09 19:10 UTC (permalink / raw)
  To: peter.maydell; +Cc: famz, qemu-devel, qemu-trivial, edgar.iglesias, patches

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL
Type: series
Message-id: 1483988759-14606-1-git-send-email-peter.maydell@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1483988759-14606-1-git-send-email-peter.maydell@linaro.org -> patchew/1483988759-14606-1-git-send-email-peter.maydell@linaro.org
Switched to a new branch 'test'
63cfa26 disas/cris.c: Fix Coverity warning about unchecked NULL

=== OUTPUT BEGIN ===
Checking PATCH 1/1: disas/cris.c: Fix Coverity warning about unchecked NULL...
ERROR: code indent should never use tabs
#24: FILE: disas/cris.c:2493:
+^Iif (sregp == NULL || sregp->name == NULL)$

ERROR: suspect code indent for conditional statements (8, 10)
#24: FILE: disas/cris.c:2493:
+	if (sregp == NULL || sregp->name == NULL)
 	  /* Should have been caught as a non-match earlier.  */

ERROR: braces {} are necessary for all arms of this statement
#24: FILE: disas/cris.c:2493:
+	if (sregp == NULL || sregp->name == NULL)
[...]

total: 3 errors, 0 warnings, 8 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL
  2017-01-09 19:05 [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL Peter Maydell
  2017-01-09 19:10 ` no-reply
@ 2017-01-09 20:13 ` Edgar E. Iglesias
  2017-01-12 11:03 ` Michael Tokarev
  2 siblings, 0 replies; 6+ messages in thread
From: Edgar E. Iglesias @ 2017-01-09 20:13 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, qemu-trivial, patches

On Mon, Jan 09, 2017 at 07:05:59PM +0000, Peter Maydell wrote:
> Coverity (CID 1005689) warns that we don't check that
> spec_reg_info() returned non-NULL before dereferencing.
> Add the check, though as the comment notes this is
> a can't-really-happen case because the earlier constraint
> matching should have ruled out the "unknown reg" case.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  disas/cris.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/disas/cris.c b/disas/cris.c
> index 08161d1..8a1daf9 100644
> --- a/disas/cris.c
> +++ b/disas/cris.c
> @@ -2490,7 +2490,7 @@ print_with_operands (const struct cris_opcode *opcodep,
>  	const struct cris_spec_reg *sregp
>  	  = spec_reg_info ((insn >> 12) & 15, disdata->distype);
>  
> -	if (sregp->name == NULL)
> +	if (sregp == NULL || sregp->name == NULL)
>  	  /* Should have been caught as a non-match earlier.  */
>  	  *tp++ = '?';
>  	else
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL
  2017-01-09 19:10 ` no-reply
@ 2017-01-09 21:35   ` Peter Maydell
  2017-01-09 22:20     ` Edgar E. Iglesias
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2017-01-09 21:35 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Fam Zheng, QEMU Trivial, Edgar E. Iglesias, patches@linaro.org

On 9 January 2017 at 19:10,  <no-reply@patchew.org> wrote:
> Checking PATCH 1/1: disas/cris.c: Fix Coverity warning about unchecked NULL...
> ERROR: code indent should never use tabs
> #24: FILE: disas/cris.c:2493:
> +^Iif (sregp == NULL || sregp->name == NULL)$
>
> ERROR: suspect code indent for conditional statements (8, 10)
> #24: FILE: disas/cris.c:2493:
> +       if (sregp == NULL || sregp->name == NULL)
>           /* Should have been caught as a non-match earlier.  */
>
> ERROR: braces {} are necessary for all arms of this statement
> #24: FILE: disas/cris.c:2493:
> +       if (sregp == NULL || sregp->name == NULL)
> [...]
>
> total: 3 errors, 0 warnings, 8 lines checked

This is because the whole file is GNU coding standards
style, being a binutils import. Better to stick with it rather
than rework, I think.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL
  2017-01-09 21:35   ` Peter Maydell
@ 2017-01-09 22:20     ` Edgar E. Iglesias
  0 siblings, 0 replies; 6+ messages in thread
From: Edgar E. Iglesias @ 2017-01-09 22:20 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Fam Zheng, QEMU Trivial, patches@linaro.org

On Mon, Jan 09, 2017 at 09:35:16PM +0000, Peter Maydell wrote:
> On 9 January 2017 at 19:10,  <no-reply@patchew.org> wrote:
> > Checking PATCH 1/1: disas/cris.c: Fix Coverity warning about unchecked NULL...
> > ERROR: code indent should never use tabs
> > #24: FILE: disas/cris.c:2493:
> > +^Iif (sregp == NULL || sregp->name == NULL)$
> >
> > ERROR: suspect code indent for conditional statements (8, 10)
> > #24: FILE: disas/cris.c:2493:
> > +       if (sregp == NULL || sregp->name == NULL)
> >           /* Should have been caught as a non-match earlier.  */
> >
> > ERROR: braces {} are necessary for all arms of this statement
> > #24: FILE: disas/cris.c:2493:
> > +       if (sregp == NULL || sregp->name == NULL)
> > [...]
> >
> > total: 3 errors, 0 warnings, 8 lines checked
> 
> This is because the whole file is GNU coding standards
> style, being a binutils import. Better to stick with it rather
> than rework, I think.

Yes, I agree.

Cheers,
Edgar

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL
  2017-01-09 19:05 [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL Peter Maydell
  2017-01-09 19:10 ` no-reply
  2017-01-09 20:13 ` Edgar E. Iglesias
@ 2017-01-12 11:03 ` Michael Tokarev
  2 siblings, 0 replies; 6+ messages in thread
From: Michael Tokarev @ 2017-01-12 11:03 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, qemu-trivial; +Cc: Edgar E. Iglesias, patches

09.01.2017 22:05, Peter Maydell wrote:
> Coverity (CID 1005689) warns that we don't check that
> spec_reg_info() returned non-NULL before dereferencing.
> Add the check, though as the comment notes this is
> a can't-really-happen case because the earlier constraint
> matching should have ruled out the "unknown reg" case.

Applied to -trivial, thanks!

/mjt

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-12 11:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-09 19:05 [Qemu-devel] [PATCH] disas/cris.c: Fix Coverity warning about unchecked NULL Peter Maydell
2017-01-09 19:10 ` no-reply
2017-01-09 21:35   ` Peter Maydell
2017-01-09 22:20     ` Edgar E. Iglesias
2017-01-09 20:13 ` Edgar E. Iglesias
2017-01-12 11:03 ` Michael Tokarev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).