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

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

[Alexey Kardashevskiy]

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 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 *)&ip_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 *)&ip_addr));
 
     /* If broadcast address */
     if (ip_addr == 0xffffffff || 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(&addr, 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 *)&haddr));
+	DEBUG_ARG("hport = %d", ntohs(hport));
+	DEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)&laddr));
+	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

Re: [Qemu-trivial] [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-aik@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 *)&haddr));$

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 *)&laddr));$

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-devel@freelists.org

Re: [Qemu-trivial] [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 <aik@ozlabs.ru>

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 *)&ip_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 *)&ip_addr));
>  
>      /* If broadcast address */
>      if (ip_addr == 0xffffffff || 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(&addr, 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 *)&haddr));
> +	DEBUG_ARG("hport = %d", ntohs(hport));
> +	DEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)&laddr));
> +	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-trivial] [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 <aik@ozlabs.ru>
> 
> 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 *)&ip_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-trivial] [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 <aik@ozlabs.ru>
>>
>> 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 *)&ip_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-trivial] [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 <aik@ozlabs.ru>
> ---
>  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 *)&ip_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 *)&ip_addr));
>  
>      /* If broadcast address */
>      if (ip_addr == 0xffffffff || 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(&addr, 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 *)&haddr));
> +	DEBUG_ARG("hport = %d", ntohs(hport));
> +	DEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)&laddr));
> +	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-trivial] [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 *)&ip_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