Re: [Qemu-devel] [PULL 0/6] Net patches

[Thomas Huth]

On 29.01.2018 11:02, Jason Wang wrote:
> 
> 
> On 2018年01月29日 16:25, address@hidden wrote:
>> Hi,
>>
>> This series seems to have some coding style problems. See output below
>> for
>> more information:
>>
>> Type: series
>> Message-id: address@hidden
>> Subject: [Qemu-devel] [PULL 0/6] Net patches
>>
>> === 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
>>
>> 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/address@hidden ->
>> patchew/address@hidden
>> Switched to a new branch 'test'
>> f599bb830a MAINTAINERS: update Dmitry Fleytman email
>> d7e337961c qemu-doc: Get rid of "vlan=X" example in the documentation
>> b7bc06c0f4 net: Allow netdevs to be used with 'hostfwd_add' and
>> 'hostfwd_remove'
>> 85b4cac4ac net: Allow hubports to connect to other netdevs
>> b4a35413c4 colo: compare the packet based on the tcp sequence number
>> f6781da302 colo: modified the payload compare function
>>
>> === OUTPUT BEGIN ===
>> Checking PATCH 1/6: colo: modified the payload compare function...
>> Checking PATCH 2/6: colo: compare the packet based on the tcp sequence
>> number...
>> Checking PATCH 3/6: net: Allow hubports to connect to other netdevs...
>> Checking PATCH 4/6: net: Allow netdevs to be used with 'hostfwd_add'
>> and 'hostfwd_remove'...
>> ERROR: consider using qemu_strtol in preference to strtol
>> #72: FILE: net/slirp.c:414:
>> +            nc = net_hub_find_client_by_name(strtol(hub_id, NULL, 0),
>> name);
>>
>> total: 1 errors, 0 warnings, 73 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.
>>
>> Checking PATCH 5/6: qemu-doc: Get rid of "vlan=X" example in the
>> documentation...
>> Checking PATCH 6/6: MAINTAINERS: update Dmitry Fleytman email...
>> === OUTPUT END ===
>>
>> Test command exited with code: 1
> 
> Thomas, want to send a v2 for this patch?

No, that strtol was already there in the code before my patch, I just
changed the indentation. If we decide that we want to replace it, this
should be done by a separate patch instead. Question is: Do we really
want to replace it here? (the error message is saying "consider ..." -
so maybe that should also rather be a warning instead of an error message?)

 Thomas