Re: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form

[Samuel Thibault]

Hello,

Remember to Cc the maintainer, I just can't read qemu-devel fully to
find slirp mails.

Thomas Huth, on mer. 07 mars 2018 07:24:16 +0100, wrote:
> >> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> >> index 3547043..bac608f 100644
> >> --- a/slirp/arp_table.c
> >> +++ b/slirp/arp_table.c
> >> @@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, 
> >> uint8_t ethaddr[ETH_ALEN])
> >>  int i;
> >>  
> >>  DEBUG_CALL("arp_table_add");
> >> -DEBUG_ARG("ip = 0x%x", ip_addr);
> >> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));

I never like casts :)
And it happens that the standard doesn't say that s_addr is necessarily
the first field of struct in_addr, so better really initialize a struct
in_addr variable and use that (ditto for arp_table_search and
tcp_listen).

Samuel

Re: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form

[Paolo Bonzini]

On 01/02/2018 10:35, Alexey Kardashevskiy wrote:
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  slirp/arp_table.c | 4 ++--
>  slirp/socket.c| 8 
>  slirp/udp.c   | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)

Samuel, Jason, can you pick this up?

Thanks,

Paolo

> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> index 3547043..bac608f 100644
> --- a/slirp/arp_table.c
> +++ b/slirp/arp_table.c
> @@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t 
> ethaddr[ETH_ALEN])
>  int i;
>  
>  DEBUG_CALL("arp_table_add");
> -DEBUG_ARG("ip = 0x%x", ip_addr);
> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));
>  DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
>  ethaddr[0], ethaddr[1], ethaddr[2],
>  ethaddr[3], ethaddr[4], ethaddr[5]));
> @@ -67,7 +67,7 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
>  int i;
>  
>  DEBUG_CALL("arp_table_search");
> -DEBUG_ARG("ip = 0x%x", ip_addr);
> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));
>  
>  /* If broadcast address */
>  if (ip_addr == 0x || ip_addr == broadcast_addr) {
> diff --git a/slirp/socket.c b/slirp/socket.c
> index cb7b5b6..61347d1 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -701,10 +701,10 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, 
> uint32_t laddr,
>   memset(, 0, addrlen);
>  
>   DEBUG_CALL("tcp_listen");
> - DEBUG_ARG("haddr = %x", haddr);
> - DEBUG_ARG("hport = %d", hport);
> - DEBUG_ARG("laddr = %x", laddr);
> - DEBUG_ARG("lport = %d", lport);
> + DEBUG_ARG("haddr = %s", inet_ntoa(*(struct in_addr *)));
> + DEBUG_ARG("hport = %d", ntohs(hport));
> + DEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)));
> + DEBUG_ARG("lport = %d", ntohs(lport));
>   DEBUG_ARG("flags = %x", flags);
>  
>   so = socreate(slirp);
> diff --git a/slirp/udp.c b/slirp/udp.c
> index 227d779..e5bf065 100644
> --- a/slirp/udp.c
> +++ b/slirp/udp.c
> @@ -241,8 +241,8 @@ int udp_output(struct socket *so, struct mbuf *m,
>   DEBUG_CALL("udp_output");
>   DEBUG_ARG("so = %p", so);
>   DEBUG_ARG("m = %p", m);
> - DEBUG_ARG("saddr = %lx", (long)saddr->sin_addr.s_addr);
> - DEBUG_ARG("daddr = %lx", (long)daddr->sin_addr.s_addr);
> + DEBUG_ARG("saddr = %s", inet_ntoa(saddr->sin_addr));
> + DEBUG_ARG("daddr = %s", inet_ntoa(daddr->sin_addr));
>  
>   /*
>* Adjust for header
> 

Re: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form

[Alexey Kardashevskiy]

On 7/3/18 5:24 pm, Thomas Huth wrote:
> On 07.03.2018 04:38, Alexey Kardashevskiy wrote:
>> On 01/02/18 20:35, Alexey Kardashevskiy wrote:
>>> Signed-off-by: Alexey Kardashevskiy 
>>
>> Ping?
>>
>>
>>> ---
>>>  slirp/arp_table.c | 4 ++--
>>>  slirp/socket.c| 8 
>>>  slirp/udp.c   | 4 ++--
>>>  3 files changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
>>> index 3547043..bac608f 100644
>>> --- a/slirp/arp_table.c
>>> +++ b/slirp/arp_table.c
>>> @@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, 
>>> uint8_t ethaddr[ETH_ALEN])
>>>  int i;
>>>  
>>>  DEBUG_CALL("arp_table_add");
>>> -DEBUG_ARG("ip = 0x%x", ip_addr);
>>> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));
> 
> Is this endianness safe? The man-page of inet_ntoa says that the
> function is expecting network byte order, so I wonder whether this works
> right on both, big and little endian hosts?



arp_table_add() is called for either sin_addr (network order) or
slirp_arphdr::ar_sip which is initialized from sin_addr (network order)
with no order conversion. Bugs are still possible, of course :)


-- 
Alexey

Re: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form

[Thomas Huth]

On 07.03.2018 04:38, Alexey Kardashevskiy wrote:
> On 01/02/18 20:35, Alexey Kardashevskiy wrote:
>> Signed-off-by: Alexey Kardashevskiy 
> 
> Ping?
> 
> 
>> ---
>>  slirp/arp_table.c | 4 ++--
>>  slirp/socket.c| 8 
>>  slirp/udp.c   | 4 ++--
>>  3 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
>> index 3547043..bac608f 100644
>> --- a/slirp/arp_table.c
>> +++ b/slirp/arp_table.c
>> @@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t 
>> ethaddr[ETH_ALEN])
>>  int i;
>>  
>>  DEBUG_CALL("arp_table_add");
>> -DEBUG_ARG("ip = 0x%x", ip_addr);
>> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));

Is this endianness safe? The man-page of inet_ntoa says that the
function is expecting network byte order, so I wonder whether this works
right on both, big and little endian hosts?

 Thomas

Re: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form

[Alexey Kardashevskiy]

On 01/02/18 20:35, Alexey Kardashevskiy wrote:
> Signed-off-by: Alexey Kardashevskiy 

Ping?


> ---
>  slirp/arp_table.c | 4 ++--
>  slirp/socket.c| 8 
>  slirp/udp.c   | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> index 3547043..bac608f 100644
> --- a/slirp/arp_table.c
> +++ b/slirp/arp_table.c
> @@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t 
> ethaddr[ETH_ALEN])
>  int i;
>  
>  DEBUG_CALL("arp_table_add");
> -DEBUG_ARG("ip = 0x%x", ip_addr);
> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));
>  DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
>  ethaddr[0], ethaddr[1], ethaddr[2],
>  ethaddr[3], ethaddr[4], ethaddr[5]));
> @@ -67,7 +67,7 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
>  int i;
>  
>  DEBUG_CALL("arp_table_search");
> -DEBUG_ARG("ip = 0x%x", ip_addr);
> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));
>  
>  /* If broadcast address */
>  if (ip_addr == 0x || ip_addr == broadcast_addr) {
> diff --git a/slirp/socket.c b/slirp/socket.c
> index cb7b5b6..61347d1 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -701,10 +701,10 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, 
> uint32_t laddr,
>   memset(, 0, addrlen);
>  
>   DEBUG_CALL("tcp_listen");
> - DEBUG_ARG("haddr = %x", haddr);
> - DEBUG_ARG("hport = %d", hport);
> - DEBUG_ARG("laddr = %x", laddr);
> - DEBUG_ARG("lport = %d", lport);
> + DEBUG_ARG("haddr = %s", inet_ntoa(*(struct in_addr *)));
> + DEBUG_ARG("hport = %d", ntohs(hport));
> + DEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)));
> + DEBUG_ARG("lport = %d", ntohs(lport));
>   DEBUG_ARG("flags = %x", flags);
>  
>   so = socreate(slirp);
> diff --git a/slirp/udp.c b/slirp/udp.c
> index 227d779..e5bf065 100644
> --- a/slirp/udp.c
> +++ b/slirp/udp.c
> @@ -241,8 +241,8 @@ int udp_output(struct socket *so, struct mbuf *m,
>   DEBUG_CALL("udp_output");
>   DEBUG_ARG("so = %p", so);
>   DEBUG_ARG("m = %p", m);
> - DEBUG_ARG("saddr = %lx", (long)saddr->sin_addr.s_addr);
> - DEBUG_ARG("daddr = %lx", (long)daddr->sin_addr.s_addr);
> + DEBUG_ARG("saddr = %s", inet_ntoa(saddr->sin_addr));
> + DEBUG_ARG("daddr = %s", inet_ntoa(daddr->sin_addr));
>  
>   /*
>* Adjust for header
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form

[no-reply]

Hi,

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

Type: series
Message-id: 20180201093545.33741-1-...@ozlabs.ru
Subject: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human 
readable form

=== 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'
2d5b0a923e slirp/debug: Print IP addresses in human readable form

=== OUTPUT BEGIN ===
Checking PATCH 1/1: slirp/debug: Print IP addresses in human readable form...
ERROR: code indent should never use tabs
#43: FILE: slirp/socket.c:704:
+^IDEBUG_ARG("haddr = %s", inet_ntoa(*(struct in_addr *)));$

ERROR: code indent should never use tabs
#44: FILE: slirp/socket.c:705:
+^IDEBUG_ARG("hport = %d", ntohs(hport));$

ERROR: code indent should never use tabs
#45: FILE: slirp/socket.c:706:
+^IDEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)));$

ERROR: code indent should never use tabs
#46: FILE: slirp/socket.c:707:
+^IDEBUG_ARG("lport = %d", ntohs(lport));$

ERROR: code indent should never use tabs
#60: FILE: slirp/udp.c:244:
+^IDEBUG_ARG("saddr = %s", inet_ntoa(saddr->sin_addr));$

ERROR: code indent should never use tabs
#61: FILE: slirp/udp.c:245:
+^IDEBUG_ARG("daddr = %s", inet_ntoa(daddr->sin_addr));$

total: 6 errors, 0 warnings, 40 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-de...@freelists.org

[Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form

[Alexey Kardashevskiy]

Signed-off-by: Alexey Kardashevskiy 
---
 slirp/arp_table.c | 4 ++--
 slirp/socket.c| 8 
 slirp/udp.c   | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index 3547043..bac608f 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t 
ethaddr[ETH_ALEN])
 int i;
 
 DEBUG_CALL("arp_table_add");
-DEBUG_ARG("ip = 0x%x", ip_addr);
+DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));
 DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
 ethaddr[0], ethaddr[1], ethaddr[2],
 ethaddr[3], ethaddr[4], ethaddr[5]));
@@ -67,7 +67,7 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
 int i;
 
 DEBUG_CALL("arp_table_search");
-DEBUG_ARG("ip = 0x%x", ip_addr);
+DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)_addr));
 
 /* If broadcast address */
 if (ip_addr == 0x || ip_addr == broadcast_addr) {
diff --git a/slirp/socket.c b/slirp/socket.c
index cb7b5b6..61347d1 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -701,10 +701,10 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, 
uint32_t laddr,
memset(, 0, addrlen);
 
DEBUG_CALL("tcp_listen");
-   DEBUG_ARG("haddr = %x", haddr);
-   DEBUG_ARG("hport = %d", hport);
-   DEBUG_ARG("laddr = %x", laddr);
-   DEBUG_ARG("lport = %d", lport);
+   DEBUG_ARG("haddr = %s", inet_ntoa(*(struct in_addr *)));
+   DEBUG_ARG("hport = %d", ntohs(hport));
+   DEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)));
+   DEBUG_ARG("lport = %d", ntohs(lport));
DEBUG_ARG("flags = %x", flags);
 
so = socreate(slirp);
diff --git a/slirp/udp.c b/slirp/udp.c
index 227d779..e5bf065 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -241,8 +241,8 @@ int udp_output(struct socket *so, struct mbuf *m,
DEBUG_CALL("udp_output");
DEBUG_ARG("so = %p", so);
DEBUG_ARG("m = %p", m);
-   DEBUG_ARG("saddr = %lx", (long)saddr->sin_addr.s_addr);
-   DEBUG_ARG("daddr = %lx", (long)daddr->sin_addr.s_addr);
+   DEBUG_ARG("saddr = %s", inet_ntoa(saddr->sin_addr));
+   DEBUG_ARG("daddr = %s", inet_ntoa(daddr->sin_addr));
 
/*
 * Adjust for header
-- 
2.11.0