[Qemu-devel] [PATCH] slirp: fix big/little endian conversion in ident protocol

[Qemu-devel] [PATCH] slirp: fix big/little endian conversion in ident protocol

[Samuel Thibault]

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/tcp_subr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index ef9d99c154..1d7e72dca7 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -660,10 +660,12 @@ tcp_emu(struct socket *so, struct mbuf *m)
 						    tmpso->so_fport == n1) {
 							if (getsockname(tmpso->s,
 								(struct sockaddr *)&addr, &addrlen) == 0)
-							   n2 = ntohs(addr.sin_port);
+							   n2 = addr.sin_port;
 							break;
 						}
 					}
+					NTOHS(n1);
+					NTOHS(n2);
 					so_rcv->sb_cc = snprintf(so_rcv->sb_data,
 								 so_rcv->sb_datalen,
 								 "%d,%d\r\n", n1, n2);
-- 
2.20.1

Re: [Qemu-devel] [PATCH] slirp: fix big/little endian conversion in ident protocol

[Philippe Mathieu-Daudé]

Hi Samuel,

On 3/2/19 3:25 AM, Samuel Thibault wrote:
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---

I got:

Applying: slirp: fix big/little endian conversion in ident protocol
error: patch failed: slirp/tcp_subr.c:660
error: slirp/tcp_subr.c: patch does not apply

Then noticed another slirp patch, applied first then this one succeed,
so you missed:

Based-on: <1551476756-25749-1-git-send-email-will@wbowling.info>

>  slirp/tcp_subr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
> index ef9d99c154..1d7e72dca7 100644
> --- a/slirp/tcp_subr.c
> +++ b/slirp/tcp_subr.c
> @@ -660,10 +660,12 @@ tcp_emu(struct socket *so, struct mbuf *m)
>  						    tmpso->so_fport == n1) {
>  							if (getsockname(tmpso->s,
>  								(struct sockaddr *)&addr, &addrlen) == 0)
> -							   n2 = ntohs(addr.sin_port);
> +							   n2 = addr.sin_port;
>  							break;
>  						}
>  					}
> +					NTOHS(n1);
> +					NTOHS(n2);
>  					so_rcv->sb_cc = snprintf(so_rcv->sb_data,
>  								 so_rcv->sb_datalen,
>  								 "%d,%d\r\n", n1, n2);
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH] slirp: fix big/little endian conversion in ident protocol

[Samuel Thibault]

Philippe Mathieu-Daudé, le sam. 02 mars 2019 18:31:56 +0100, a ecrit:
> On 3/2/19 3:25 AM, Samuel Thibault wrote:
> > Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> > ---
> 
> I got:
> 
> Applying: slirp: fix big/little endian conversion in ident protocol
> error: patch failed: slirp/tcp_subr.c:660
> error: slirp/tcp_subr.c: patch does not apply
> 
> Then noticed another slirp patch, applied first then this one succeed,
> so you missed:
> 
> Based-on: <1551476756-25749-1-git-send-email-will@wbowling.info>

Oh, I didn't know about that one.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!

Samuel

Re: [Qemu-devel] [PATCH] slirp: fix big/little endian conversion in ident protocol

[Philippe Mathieu-Daudé]

On 3/2/19 6:36 PM, Samuel Thibault wrote:
> Philippe Mathieu-Daudé, le sam. 02 mars 2019 18:31:56 +0100, a ecrit:
>> On 3/2/19 3:25 AM, Samuel Thibault wrote:
>>> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
>>> ---
>>
>> I got:
>>
>> Applying: slirp: fix big/little endian conversion in ident protocol
>> error: patch failed: slirp/tcp_subr.c:660
>> error: slirp/tcp_subr.c: patch does not apply
>>
>> Then noticed another slirp patch, applied first then this one succeed,
>> so you missed:
>>
>> Based-on: <1551476756-25749-1-git-send-email-will@wbowling.info>
> 
> Oh, I didn't know about that one.

BTW 'Based-on' is a hint for the reviewers and automatic testing bots,
telling "this commit is <based on> this other <series|patch>", and
without applying the base it is unlikely this patch|series applies.

It is not meant the git history, so for single commit we add it under
the first '---' separator (automatically stripped out when applied) or
for series we add it to the cover.

I guess there are no doc about that hint in HACKING neither in the wiki,
I remember the post where it was introduced:
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01288.html

Regards,

Phil.

> 
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Thanks!
> 
> Samuel
>