mbox series

[0/2] slirp: handle errors in sosendoob()

Message ID 1496679576-14336-1-git-send-email-peter.maydell@linaro.org
Headers show
Series slirp: handle errors in sosendoob() | expand

Message

Peter Maydell June 5, 2017, 4:19 p.m. UTC
At the moment the slirp sosendoob() function doesn't properly
handle errors from slirp_send(), and its callers don't do
anything with its return value either. (Coverity spots the
latter as CID 1005633.)

This patchset attempts to fix that. In the first patch we
fix sosendoob() itself so that we return errors to the caller
and treat short writes as reducing the amount of urgent data
to be sent. In the second patch we make the call in sowrite()
treat any return form sosendoob() which didn't transfer all
the urgent data as a "disconnect the socket" error in the
same way that it already handles errors for writes of
non-urgent data. The call to sosendoob() in sbappend() can
safely ignore any failures as in this case sowrite() will be
called shortly and will deal with them.

Coding style: I've opted to stick with the tab-indent
that the slirp code uses rather than reindent to QEMU standard.
Happy to go the other way if the maintainer prefers.

Disclaimer: I have not tested these patches very hard and
in particular they deal with error paths and with urgent
data, neither of which I have any test case for...

thanks
-- PMM

Peter Maydell (2):
  slirp: Handle error returns from slirp_send() in sosendoob()
  slirp: Handle error returns from sosendoob()

 slirp/sbuf.c   |  2 +-
 slirp/socket.c | 51 ++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 35 insertions(+), 18 deletions(-)

-- 
2.7.4

Comments

no-reply@patchew.org June 5, 2017, 6:22 p.m. UTC | #1
Hi,

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

Message-id: 1496679576-14336-1-git-send-email-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH 0/2] slirp: handle errors in sosendoob()
Type: series

=== 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
Switched to a new branch 'test'
9af1ed0 slirp: Handle error returns from sosendoob()
ba36c86 slirp: Handle error returns from slirp_send() in sosendoob()

=== OUTPUT BEGIN ===
Checking PATCH 1/2: slirp: Handle error returns from slirp_send() in sosendoob()...
ERROR: code indent should never use tabs
#33: FILE: slirp/socket.c:354:
+^I^Iuint32_t urgc = so->so_urgc;$

ERROR: code indent should never use tabs
#36: FILE: slirp/socket.c:356:
+^I^Iif (len > urgc) {$

ERROR: code indent should never use tabs
#37: FILE: slirp/socket.c:357:
+^I^I^Ilen = urgc;$

ERROR: code indent should never use tabs
#38: FILE: slirp/socket.c:358:
+^I^I}$

ERROR: code indent should never use tabs
#42: FILE: slirp/socket.c:360:
+^I^Iurgc -= len;$

ERROR: code indent should never use tabs
#43: FILE: slirp/socket.c:361:
+^I^Iif (urgc) {$

ERROR: code indent should never use tabs
#46: FILE: slirp/socket.c:363:
+^I^I^Iif (n > urgc) {$

ERROR: code indent should never use tabs
#47: FILE: slirp/socket.c:364:
+^I^I^I^In = urgc;$

ERROR: code indent should never use tabs
#48: FILE: slirp/socket.c:365:
+^I^I^I}$

ERROR: code indent should never use tabs
#54: FILE: slirp/socket.c:370:
+^I}$

ERROR: code indent should never use tabs
#59: FILE: slirp/socket.c:373:
+^Iif (n != len) {$

ERROR: code indent should never use tabs
#60: FILE: slirp/socket.c:374:
+^I^IDEBUG_ERROR((dfd, "Didn't send all data urgently XXXXX\n"));$

ERROR: code indent should never use tabs
#61: FILE: slirp/socket.c:375:
+^I}$

ERROR: code indent should never use tabs
#64: FILE: slirp/socket.c:377:
+^Iif (n < 0) {$

ERROR: code indent should never use tabs
#65: FILE: slirp/socket.c:378:
+^I^Ireturn n;$

ERROR: code indent should never use tabs
#67: FILE: slirp/socket.c:380:
+^Iso->so_urgc -= n;$

ERROR: line over 90 characters
#68: FILE: slirp/socket.c:381:
+	DEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent bytes left\n", n, so->so_urgc));

ERROR: code indent should never use tabs
#68: FILE: slirp/socket.c:381:
+^IDEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent bytes left\n", n, so->so_urgc));$

total: 18 errors, 0 warnings, 51 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 2/2: slirp: Handle error returns from sosendoob()...
ERROR: code indent should never use tabs
#25: FILE: slirp/sbuf.c:94:
+^I^I(void)sosendoob(so);$

ERROR: code indent should never use tabs
#38: FILE: slirp/socket.c:407:
+^I^Iif (sosendoob(so) < so->so_urgc) {$

ERROR: code indent should never use tabs
#39: FILE: slirp/socket.c:408:
+^I^I^I/* Treat a short write as a fatal error too,$

ERROR: code indent should never use tabs
#40: FILE: slirp/socket.c:409:
+^I^I^I * rather than continuing on and sending the urgent$

ERROR: code indent should never use tabs
#41: FILE: slirp/socket.c:410:
+^I^I^I * data as if it were non-urgent and leaving the$

ERROR: code indent should never use tabs
#42: FILE: slirp/socket.c:411:
+^I^I^I * so_urgc count wrong.$

ERROR: code indent should never use tabs
#43: FILE: slirp/socket.c:412:
+^I^I^I */$

ERROR: code indent should never use tabs
#44: FILE: slirp/socket.c:413:
+^I^I^Igoto err_disconnected;$

ERROR: code indent should never use tabs
#45: FILE: slirp/socket.c:414:
+^I^I}$

ERROR: code indent should never use tabs
#58: FILE: slirp/socket.c:458:
+^I^Igoto err_disconnected;$

WARNING: line over 80 characters
#68: FILE: slirp/socket.c:487:
+	DEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x, errno = %d\n",

ERROR: code indent should never use tabs
#68: FILE: slirp/socket.c:487:
+^IDEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x, errno = %d\n",$

ERROR: code indent should never use tabs
#69: FILE: slirp/socket.c:488:
+^I^I    so->so_state, errno));$

ERROR: code indent should never use tabs
#70: FILE: slirp/socket.c:489:
+^Isofcantsendmore(so);$

ERROR: code indent should never use tabs
#71: FILE: slirp/socket.c:490:
+^Itcp_sockclosed(sototcpcb(so));$

ERROR: code indent should never use tabs
#72: FILE: slirp/socket.c:491:
+^Ireturn -1;$

total: 15 errors, 1 warnings, 48 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
Peter Maydell June 26, 2017, 12:24 p.m. UTC | #2
Ping for review?

thanks
-- PMM

On 5 June 2017 at 17:19, Peter Maydell <peter.maydell@linaro.org> wrote:
> At the moment the slirp sosendoob() function doesn't properly

> handle errors from slirp_send(), and its callers don't do

> anything with its return value either. (Coverity spots the

> latter as CID 1005633.)

>

> This patchset attempts to fix that. In the first patch we

> fix sosendoob() itself so that we return errors to the caller

> and treat short writes as reducing the amount of urgent data

> to be sent. In the second patch we make the call in sowrite()

> treat any return form sosendoob() which didn't transfer all

> the urgent data as a "disconnect the socket" error in the

> same way that it already handles errors for writes of

> non-urgent data. The call to sosendoob() in sbappend() can

> safely ignore any failures as in this case sowrite() will be

> called shortly and will deal with them.

>

> Coding style: I've opted to stick with the tab-indent

> that the slirp code uses rather than reindent to QEMU standard.

> Happy to go the other way if the maintainer prefers.

>

> Disclaimer: I have not tested these patches very hard and

> in particular they deal with error paths and with urgent

> data, neither of which I have any test case for...

>

> thanks

> -- PMM

>

> Peter Maydell (2):

>   slirp: Handle error returns from slirp_send() in sosendoob()

>   slirp: Handle error returns from sosendoob()

>

>  slirp/sbuf.c   |  2 +-

>  slirp/socket.c | 51 ++++++++++++++++++++++++++++++++++-----------------

>  2 files changed, 35 insertions(+), 18 deletions(-)
Peter Maydell July 9, 2017, 9:21 p.m. UTC | #3
Ping^2 ?

thanks
-- PMM

On 26 June 2017 at 13:24, Peter Maydell <peter.maydell@linaro.org> wrote:
> Ping for review?

>

> thanks

> -- PMM

>

> On 5 June 2017 at 17:19, Peter Maydell <peter.maydell@linaro.org> wrote:

>> At the moment the slirp sosendoob() function doesn't properly

>> handle errors from slirp_send(), and its callers don't do

>> anything with its return value either. (Coverity spots the

>> latter as CID 1005633.)

>>

>> This patchset attempts to fix that. In the first patch we

>> fix sosendoob() itself so that we return errors to the caller

>> and treat short writes as reducing the amount of urgent data

>> to be sent. In the second patch we make the call in sowrite()

>> treat any return form sosendoob() which didn't transfer all

>> the urgent data as a "disconnect the socket" error in the

>> same way that it already handles errors for writes of

>> non-urgent data. The call to sosendoob() in sbappend() can

>> safely ignore any failures as in this case sowrite() will be

>> called shortly and will deal with them.

>>

>> Coding style: I've opted to stick with the tab-indent

>> that the slirp code uses rather than reindent to QEMU standard.

>> Happy to go the other way if the maintainer prefers.

>>

>> Disclaimer: I have not tested these patches very hard and

>> in particular they deal with error paths and with urgent

>> data, neither of which I have any test case for...

>>

>> thanks

>> -- PMM

>>

>> Peter Maydell (2):

>>   slirp: Handle error returns from slirp_send() in sosendoob()

>>   slirp: Handle error returns from sosendoob()

>>

>>  slirp/sbuf.c   |  2 +-

>>  slirp/socket.c | 51 ++++++++++++++++++++++++++++++++++-----------------

>>  2 files changed, 35 insertions(+), 18 deletions(-)
Samuel Thibault July 9, 2017, 9:56 p.m. UTC | #4
Peter Maydell, on dim. 09 juil. 2017 22:21:01 +0100, wrote:
> Ping^2 ?


I'm sorry I'm still too busy ATM, it's still far in my mbox.

Samuel