Re: [Qemu-devel] [PATCH 2/3] linux-user: add SO_LINGER to setsockopt

[Carlo Arenas]

On Wed, Sep 20, 2017 at 11:53 AM, Laurent Vivier <address@hidden> wrote:

> > the test for optlen is replaced by passing optlen to the underlying
> > setsockopt call directly, who would do the test and return the right
> error.
>
> You can't do that, because sizeof(struct linger) may be different from
> sizeof(struct target_linger).
>

Good point, will correct it, but considering that was mostly what I changed
from 陈刚's code, could we merge his instead so I can rebase my changes on
top of it?

just out of curiosity, do you know any such architecture? I assumed that
for everything qemu will care, a struct with 2 ints would be 8 bytes long.


> > as an interesting note, I noticed when testing (in ubuntu artful x86_64)
> > that regardless of how you interpret the documentation, setsockopt won't
> > fail just because the len is smaller than the size of the struct, and
>
> Right, see:
>
> http://elixir.free-electrons.com/linux/latest/source/net/core/sock.c#L830


Sorry; got confused and the one that doesn't fail is actually getsockopt:

http://elixir.free-electrons.com/linux/latest/source/net/core/sock.c#L1178

> therefore that code was not equivalent to the setsockopt it was trying
> > to emulate, and therefore this change doesn't only make the code simpler
> > but also more correct IMHO
> Next time add a revision history in your series explaining your changes
> (and don't reply to the previous patch series for the new series, it's
> better to start a new email thread).
>

Sorry about that, my original intent was to get the original submission to
add support of SO_LINGER to setsockopt out of patchwork limbo[1], hence the
threading and inherited CC

I see there is a lot more work to be done here though, specially when I
found out while trying to test my change for sparc that SOL_SOCKET was also
wrong[2]

is there any testing infrastructure that could be used here to make sure
that no regression is introduced?

Carlo

[1] https://patchwork.ozlabs.org/patch/565659/
[2] https://patchwork.ozlabs.org/patch/816043/