I've re-run some scripts from the coccinelle directory,
and they have found some problems.

This series fixes them.

Laurent Vivier (4):
  error: Strip trailing '\n' from error string arguments (again again)
  error: Remove NULL checks on error_propagate() calls
  qdict: remove useless cast
  Remove unnecessary variables for function return value

 accel/tcg/translate-all.c                          |  5 +-
 block/nvme.c                                       | 11 ++---
 block/quorum.c                                     |  6 +--
 hw/arm/exynos4210.c                                |  7 +--
 hw/block/vhost-user-blk.c                          |  5 +-
 hw/hppa/dino.c                                     |  5 +-
 hw/misc/mos6522.c                                  |  6 +--
 hw/net/ftgmac100.c                                 |  5 +-
 hw/ppc/pnv_lpc.c                                   | 14 +-----
 io/channel-websock.c                               |  4 +-
 io/net-listener.c                                  |  6 +--
 monitor.c                                          |  2 +-
 target/i386/hax-darwin.c                           | 10 ++--
 target/i386/hvf/hvf.c                              | 24 +++++-----
 target/mips/dsp_helper.c                           | 15 ++----
 target/xtensa/core-dc232b/xtensa-modules.c         | 56 ++++++----------------
 target/xtensa/core-dc233c/xtensa-modules.c         | 56 ++++++----------------
 target/xtensa/core-de212/xtensa-modules.c          | 48 +++++--------------
 target/xtensa/core-fsf/xtensa-modules.c            | 32 ++++---------
 .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++-------
 target/xtensa/translate.c                          |  5 +-
 tests/m48t59-test.c                                |  6 +--
 tests/test-thread-pool.c                           |  6 +--
 util/uri.c                                         |  5 +-
 24 files changed, 94 insertions(+), 269 deletions(-)

-- 
2.14.3

22.03.2018 19:12, Laurent Vivier wrote:
> I've re-run some scripts from the coccinelle directory,
> and they have found some problems.
> 
> This series fixes them.
> 
> Laurent Vivier (4):
>   error: Strip trailing '\n' from error string arguments (again again)
>   error: Remove NULL checks on error_propagate() calls
>   qdict: remove useless cast
>   Remove unnecessary variables for function return value

I've applied patches 1-3, but the 4th patch is a bit.. interesting.
As has already been said, it touches auto-generated files, which is
sort of fine, but it _also_ removes comments, which - I think - is
quite a bit wrong.

I can apply just selected hunks, but.. hmm..

/mjt

On 23/03/2018 13:37, Michael Tokarev wrote:
> 22.03.2018 19:12, Laurent Vivier wrote:
>> I've re-run some scripts from the coccinelle directory,
>> and they have found some problems.
>>
>> This series fixes them.
>>
>> Laurent Vivier (4):
>>   error: Strip trailing '\n' from error string arguments (again again)
>>   error: Remove NULL checks on error_propagate() calls
>>   qdict: remove useless cast
>>   Remove unnecessary variables for function return value
> 
> I've applied patches 1-3, but the 4th patch is a bit.. interesting.
> As has already been said, it touches auto-generated files, which is
> sort of fine, but it _also_ removes comments, which - I think - is
> quite a bit wrong.
> 
> I can apply just selected hunks, but.. hmm..

Thank you, I'm going to rework this one.

Thanks,
Laurent

On 03/23/2018 07:37 AM, Michael Tokarev wrote:
> 22.03.2018 19:12, Laurent Vivier wrote:
>> I've re-run some scripts from the coccinelle directory,
>> and they have found some problems.
>>
>> This series fixes them.
>>
>> Laurent Vivier (4):
>>    error: Strip trailing '\n' from error string arguments (again again)
>>    error: Remove NULL checks on error_propagate() calls
>>    qdict: remove useless cast
>>    Remove unnecessary variables for function return value
> 
> I've applied patches 1-3, but the 4th patch is a bit.. interesting.
> As has already been said, it touches auto-generated files, which is
> sort of fine, but it _also_ removes comments, which - I think - is
> quite a bit wrong.

1-3 are also candidates for going through my qapi tree (I will probably 
do another pull request Monday or Tuesday), if that beats your trivial 
tree.  But if not, for 1-3, you can add:
Acked-by: Eric Blake <eblake@redhat.com>

> 
> I can apply just selected hunks, but.. hmm..

v2 of patch 4 is better, and the trivial tree may indeed be a better 
place for patch 4 since it doesn't really touch anything qapi-related.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 03/22/2018 11:12 AM, Laurent Vivier wrote:
> I've re-run some scripts from the coccinelle directory,
> and they have found some problems.
> 
> This series fixes them.
> 
> Laurent Vivier (4):
>    error: Strip trailing '\n' from error string arguments (again again)

This is user-visible, so appropriate during freeze as a bug fix.  It's 
closes to the error/qapi code that Markus normally maintains (and where 
I'm filling in while he is out), so I'll queue this one on my QAPI tree.

>    error: Remove NULL checks on error_propagate() calls
>    qdict: remove useless cast
>    Remove unnecessary variables for function return value

These are cosmetic; they should be no-ops.  2 and 3 are again best 
through error/qapi, while 4 is more generic, but there's little reason 
to split up the series just because 4 is not as closely related.  Unless 
anyone has an opinion otherwise, I'm planning to also include them in my 
QAPI tree for 2.12.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 03/22/2018 11:12 AM, Laurent Vivier wrote:
> I've re-run some scripts from the coccinelle directory,
> and they have found some problems.
> 
> This series fixes them.
> 
> Laurent Vivier (4):
>    error: Strip trailing '\n' from error string arguments (again again)
>    error: Remove NULL checks on error_propagate() calls
>    qdict: remove useless cast
>    Remove unnecessary variables for function return value

I've gone through all the patches to double-check for no surprises; and 
found some formatting nits on 4 that can be touched up on pull request. 
Series:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Hi,

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

Type: series
Message-id: 20180322161226.29796-1-lvivier@redhat.com
Subject: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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
 t [tag update]            patchew/20180321144107.22770-1-kwolf@redhat.com -> patchew/20180321144107.22770-1-kwolf@redhat.com
Switched to a new branch 'test'
a1c7275b57 Remove unnecessary variables for function return value
a530fa5a72 qdict: remove useless cast
c1bfe26b11 error: Remove NULL checks on error_propagate() calls
db3d55e58e error: Strip trailing '\n' from error string arguments (again again)

=== OUTPUT BEGIN ===
Checking PATCH 1/4: error: Strip trailing '\n' from error string arguments (again again)...
Checking PATCH 2/4: error: Remove NULL checks on error_propagate() calls...
Checking PATCH 3/4: qdict: remove useless cast...
Checking PATCH 4/4: Remove unnecessary variables for function return value...
ERROR: return is not a function, parentheses are not required
#251: FILE: target/mips/dsp_helper.c:3281:
+    return (temp[1] << 63) | (temp[0] >> 1);

ERROR: return is not a function, parentheses are not required
#270: FILE: target/mips/dsp_helper.c:3308:
+    return (temp[1] << 63) | (temp[0] >> 1);

ERROR: return is not a function, parentheses are not required
#289: FILE: target/mips/dsp_helper.c:3341:
+    return (temp[1] << 63) | (temp[0] >> 1);

total: 3 errors, 0 warnings, 813 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

On 22 March 2018 at 17:57,  <no-reply@patchew.org> wrote:
> Checking PATCH 4/4: Remove unnecessary variables for function return value...
> ERROR: return is not a function, parentheses are not required
> #251: FILE: target/mips/dsp_helper.c:3281:
> +    return (temp[1] << 63) | (temp[0] >> 1);

This looks like a bug in checkpatch. I guess to fix it you'd need
to make checkpatch count opening and closing parens in the line
to see if it goes to 0 somewhere other than just before the ';'...

thanks
-- PMM

On 03/22/2018 01:00 PM, Peter Maydell wrote:
> On 22 March 2018 at 17:57,  <no-reply@patchew.org> wrote:
>> Checking PATCH 4/4: Remove unnecessary variables for function return value...
>> ERROR: return is not a function, parentheses are not required
>> #251: FILE: target/mips/dsp_helper.c:3281:
>> +    return (temp[1] << 63) | (temp[0] >> 1);
> 
> This looks like a bug in checkpatch. I guess to fix it you'd need
> to make checkpatch count opening and closing parens in the line
> to see if it goes to 0 somewhere other than just before the ';'...

Or if we don't patch the false negative, you can bypass checkpatch with 
an ugly hack:

return 0 + (...) | (...);

(I'm NOT going to do that bypass - it's too ugly for my taste)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 22 March 2018 at 19:12, Eric Blake <eblake@redhat.com> wrote:
> Or if we don't patch the false negative, you can bypass checkpatch with an
> ugly hack:
>
> return 0 + (...) | (...);
>
> (I'm NOT going to do that bypass - it's too ugly for my taste)

Yeah, that's definitely not something we should be doing.
checkpatch has plenty of false positives anyway, ignoring one
more is no big deal.

thanks
-- PMM