[Qemu-devel] [PULL 0/5] slirp updates

Samuel Thibault posted 5 patches 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170226202709.2114-1-samuel.thibault@ens-lyon.org
Test checkpatch failed
Test docker passed
Test s390x passed
There is a newer version of this series
slirp/sbuf.h    |   4 +-
slirp/slirp.c   | 449 ++++++++++++++++++++++++++++----------------------------
slirp/socket.h  |  24 ++-
slirp/tcp_var.h |   6 +-
4 files changed, 238 insertions(+), 245 deletions(-)
[Qemu-devel] [PULL 0/5] slirp updates
Posted by Samuel Thibault 7 years, 1 month ago
The following changes since commit 685783c5b69c83c942d1fc21679311eeb8f79ab9:

  Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2017-02-26 16:38:40 +0000)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to c363a5b7f9ca9e802665587900b7ea1aefcf26ea:

  slirp: VMStatify remaining except for loop (2017-02-26 21:16:38 +0100)

----------------------------------------------------------------
slirp updates

----------------------------------------------------------------
Dr. David Alan Gilbert (5):
      slirp: VMState conversion; tcpcb
      slirp: VMStatify sbuf
      slirp: Common lhost/fhost union
      slirp: VMStatify socket level
      slirp: VMStatify remaining except for loop

 slirp/sbuf.h    |   4 +-
 slirp/slirp.c   | 449 ++++++++++++++++++++++++++++----------------------------
 slirp/socket.h  |  24 ++-
 slirp/tcp_var.h |   6 +-
 4 files changed, 238 insertions(+), 245 deletions(-)

Re: [Qemu-devel] [PULL 0/5] slirp updates
Posted by no-reply@patchew.org 7 years, 1 month ago
Hi,

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

Message-id: 20170226202709.2114-1-samuel.thibault@ens-lyon.org
Type: series
Subject: [Qemu-devel] [PULL 0/5] slirp updates

=== 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
 - [tag update]      patchew/20170224182844.32452-1-dgilbert@redhat.com -> patchew/20170224182844.32452-1-dgilbert@redhat.com
 - [tag update]      patchew/20170226165345.8757-1-bobby.prani@gmail.com -> patchew/20170226165345.8757-1-bobby.prani@gmail.com
 * [new tag]         patchew/20170226202709.2114-1-samuel.thibault@ens-lyon.org -> patchew/20170226202709.2114-1-samuel.thibault@ens-lyon.org
Switched to a new branch 'test'
150dc51 slirp: VMStatify remaining except for loop
0f3595f slirp: VMStatify socket level
b584f7a slirp: Common lhost/fhost union
38657f4 slirp: VMStatify sbuf
d064fb9 slirp: VMState conversion; tcpcb

=== OUTPUT BEGIN ===
Checking PATCH 1/5: slirp: VMState conversion; tcpcb...
ERROR: code indent should never use tabs
#212: FILE: slirp/tcp_var.h:51:
+^Iuint8_t t_force;^I^I/* 1 if forcing out a byte */$

ERROR: code indent should never use tabs
#222: FILE: slirp/tcp_var.h:112:
+^Iuint8_t^It_oobflags;^I^I/* have some */$

ERROR: code indent should never use tabs
#223: FILE: slirp/tcp_var.h:113:
+^Iuint8_t^It_iobc;^I^I^I/* input character */$

total: 3 errors, 0 warnings, 195 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/5: slirp: VMStatify sbuf...
ERROR: code indent should never use tabs
#26: FILE: slirp/sbuf.h:15:
+^Iuint32_t sb_cc;^I^I/* actual chars in buffer */$

ERROR: code indent should never use tabs
#27: FILE: slirp/sbuf.h:16:
+^Iuint32_t sb_datalen;^I/* Length of data  */$

total: 2 errors, 0 warnings, 155 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 3/5: slirp: Common lhost/fhost union...
Checking PATCH 4/5: slirp: VMStatify socket level...
ERROR: if this code is redundant consider removing it
#98: FILE: slirp/slirp.c:1297:
+#if 0

total: 1 errors, 0 warnings, 217 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/5: slirp: VMStatify remaining except for loop...
=== 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
Re: [Qemu-devel] [PULL 0/5] slirp updates
Posted by Peter Maydell 7 years, 1 month ago
On 26 February 2017 at 20:27, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> The following changes since commit 685783c5b69c83c942d1fc21679311eeb8f79ab9:
>
>   Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2017-02-26 16:38:40 +0000)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to c363a5b7f9ca9e802665587900b7ea1aefcf26ea:
>
>   slirp: VMStatify remaining except for loop (2017-02-26 21:16:38 +0100)
>
> ----------------------------------------------------------------
> slirp updates
>
> ----------------------------------------------------------------
> Dr. David Alan Gilbert (5):
>       slirp: VMState conversion; tcpcb
>       slirp: VMStatify sbuf
>       slirp: Common lhost/fhost union
>       slirp: VMStatify socket level
>       slirp: VMStatify remaining except for loop

I'm afraid this doesn't build on OSX:


/Users/pm215/src/qemu-for-merges/slirp/slirp.c:1291:9: error:
'uint16_t *' (aka 'unsigned short *') and 'typeof (((union
slirp_sockaddr *)0)->ss.ss_family) *' (aka 'unsigned char *') are not
pointers to compatible types
        VMSTATE_SS_FAMILY(ss.ss_family, union slirp_sockaddr),
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/pm215/src/qemu-for-merges/slirp/slirp.c:1277:33: note: expanded
from macro 'VMSTATE_SS_FAMILY'
#define VMSTATE_SS_FAMILY(f, s) VMSTATE_UINT16(f, s)
                                ^~~~~~~~~~~~~~~~~~~~
/Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:785:5:
note: expanded from macro 'VMSTATE_UINT16'
    VMSTATE_UINT16_V(_f, _s, 0)
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:764:5:
note: expanded from macro 'VMSTATE_UINT16_V'
    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint16, uint16_t)
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: (skipping 1 expansions in backtrace; use
-fmacro-backtrace-limit=0 to see all)
/Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:300:21:
note: expanded from macro 'VMSTATE_SINGLE_TEST'
    .offset       = vmstate_offset_value(_state, _field, _type),     \
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:272:6:
note: expanded from macro 'vmstate_offset_value'
     type_check(_type, typeof_field(_state, _field)))
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/pm215/src/qemu-for-merges/include/qemu/compiler.h:86:35: note:
expanded from macro 'type_check'
#define type_check(t1,t2) ((t1*)0 - (t2*)0)
                           ~~~~~~ ^ ~~~~~~
1 error generated.

In the OSX headers sockaddr_storage is:

struct sockaddr_storage {
        __uint8_t       ss_len;         /* address length */
        sa_family_t     ss_family;      /* [XSI] address family */
        char                    __ss_pad1[_SS_PAD1SIZE];
        __int64_t       __ss_align;     /* force structure storage alignment */
        char                    __ss_pad2[_SS_PAD2SIZE];
};

and sa_family_t is

typedef __uint8_t               sa_family_t;

(NetBSD also defines sa_family_t as an 8 bit type, and
perhaps so do the other BSDs.)

I think we can't get away with having the on-the-wire
type for this field be the same as the in-memory
representation, since the on-the-wire rep. should
be host-OS-independent...

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/5] slirp updates
Posted by Dr. David Alan Gilbert 7 years, 1 month ago
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 26 February 2017 at 20:27, Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
> > The following changes since commit 685783c5b69c83c942d1fc21679311eeb8f79ab9:
> >
> >   Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2017-02-26 16:38:40 +0000)
> >
> > are available in the git repository at:
> >
> >   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
> >
> > for you to fetch changes up to c363a5b7f9ca9e802665587900b7ea1aefcf26ea:
> >
> >   slirp: VMStatify remaining except for loop (2017-02-26 21:16:38 +0100)
> >
> > ----------------------------------------------------------------
> > slirp updates
> >
> > ----------------------------------------------------------------
> > Dr. David Alan Gilbert (5):
> >       slirp: VMState conversion; tcpcb
> >       slirp: VMStatify sbuf
> >       slirp: Common lhost/fhost union
> >       slirp: VMStatify socket level
> >       slirp: VMStatify remaining except for loop
> 
> I'm afraid this doesn't build on OSX:
> 
> 
> /Users/pm215/src/qemu-for-merges/slirp/slirp.c:1291:9: error:
> 'uint16_t *' (aka 'unsigned short *') and 'typeof (((union
> slirp_sockaddr *)0)->ss.ss_family) *' (aka 'unsigned char *') are not
> pointers to compatible types
>         VMSTATE_SS_FAMILY(ss.ss_family, union slirp_sockaddr),
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Yes, we need to drop 4 and 5;  as per the separate thread the problem
is the BSDs just have a char for their ss_family.

Dave

> /Users/pm215/src/qemu-for-merges/slirp/slirp.c:1277:33: note: expanded
> from macro 'VMSTATE_SS_FAMILY'
> #define VMSTATE_SS_FAMILY(f, s) VMSTATE_UINT16(f, s)
>                                 ^~~~~~~~~~~~~~~~~~~~
> /Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:785:5:
> note: expanded from macro 'VMSTATE_UINT16'
>     VMSTATE_UINT16_V(_f, _s, 0)
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> /Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:764:5:
> note: expanded from macro 'VMSTATE_UINT16_V'
>     VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint16, uint16_t)
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> note: (skipping 1 expansions in backtrace; use
> -fmacro-backtrace-limit=0 to see all)
> /Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:300:21:
> note: expanded from macro 'VMSTATE_SINGLE_TEST'
>     .offset       = vmstate_offset_value(_state, _field, _type),     \
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /Users/pm215/src/qemu-for-merges/include/migration/vmstate.h:272:6:
> note: expanded from macro 'vmstate_offset_value'
>      type_check(_type, typeof_field(_state, _field)))
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /Users/pm215/src/qemu-for-merges/include/qemu/compiler.h:86:35: note:
> expanded from macro 'type_check'
> #define type_check(t1,t2) ((t1*)0 - (t2*)0)
>                            ~~~~~~ ^ ~~~~~~
> 1 error generated.
> 
> In the OSX headers sockaddr_storage is:
> 
> struct sockaddr_storage {
>         __uint8_t       ss_len;         /* address length */
>         sa_family_t     ss_family;      /* [XSI] address family */
>         char                    __ss_pad1[_SS_PAD1SIZE];
>         __int64_t       __ss_align;     /* force structure storage alignment */
>         char                    __ss_pad2[_SS_PAD2SIZE];
> };
> 
> and sa_family_t is
> 
> typedef __uint8_t               sa_family_t;
> 
> (NetBSD also defines sa_family_t as an 8 bit type, and
> perhaps so do the other BSDs.)
> 
> I think we can't get away with having the on-the-wire
> type for this field be the same as the in-memory
> representation, since the on-the-wire rep. should
> be host-OS-independent...
> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK