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

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

[Alexey Kardashevskiy]

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

checkpatch.pl complains on every single changed line as it keeps
using tabs - do I need to post 's/\t/    /g'?

---
Changes:
v2:
* replaced static cast to (in_addr*) with temporary structs
---
 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..f81963b 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){.s_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){.s_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..318301f 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){.s_addr = haddr}));
+	DEBUG_ARG("hport = %d", ntohs(hport));
+	DEBUG_ARG("laddr = %s", inet_ntoa((struct in_addr){.s_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-devel] [PATCH qemu v2] slirp/debug: Print IP addresses in human readable form

[Samuel Thibault]

Alexey Kardashevskiy, on mar. 13 mars 2018 15:49:44 +1100, wrote:
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Applied to my tree, thanks!

> ---
> 
> checkpatch.pl complains on every single changed line as it keeps
> using tabs - do I need to post 's/\t/    /g'?
> 
> ---
> Changes:
> v2:
> * replaced static cast to (in_addr*) with temporary structs
> ---
>  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..f81963b 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){.s_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){.s_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..318301f 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){.s_addr = haddr}));
> +	DEBUG_ARG("hport = %d", ntohs(hport));
> +	DEBUG_ARG("laddr = %s", inet_ntoa((struct in_addr){.s_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
> 

-- 
Samuel
"I don't know why, but first C programs tend to look a lot worse than
first programs in any other language (maybe except for fortran, but then
I suspect all fortran programs look like `firsts')"
(By Olaf Kirch)

Re: [Qemu-devel] [PATCH qemu v2] 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: 20180313044944.21058-1-aik@ozlabs.ru
Subject: [Qemu-devel] [PATCH qemu v2] 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
git config --local diff.algorithm histogram

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'
e91d6cceb5 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){.s_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){.s_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-devel] [PATCH qemu v2] slirp/debug: Print IP addresses in human readable form

[Eric Blake]

On 03/12/2018 11:49 PM, Alexey Kardashevskiy wrote:
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> checkpatch.pl complains on every single changed line as it keeps
> using tabs - do I need to post 's/\t/    /g'?

No. checkpatch.pl is guidance, but even stronger is 'be consistent to 
what you are editing'; we know to ignore the checkpatch warnings on 
slirp code as that has historically used tabs.  Mixed style, where TABS 
occur in the context but your new additions use space, is also okay. 
(You CAN clean up the entire slirp files if you want, but get maintainer 
buy-in before doing so; and make the indentation cleanup separate from 
any other patch, so that a diff with whitespace ignored shows no change).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

[Alexey Kardashevskiy]

On 13/3/18 6:44 pm, Samuel Thibault wrote:
> Alexey Kardashevskiy, on mar. 13 mars 2018 15:49:44 +1100, wrote:
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Applied to my tree, thanks!


And what is your tree, is this something to be merged sometime later
somewhere? :)



> 
>> ---
>>
>> checkpatch.pl complains on every single changed line as it keeps
>> using tabs - do I need to post 's/\t/    /g'?
>>
>> ---
>> Changes:
>> v2:
>> * replaced static cast to (in_addr*) with temporary structs
>> ---
>>  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..f81963b 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){.s_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){.s_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..318301f 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){.s_addr = haddr}));
>> +	DEBUG_ARG("hport = %d", ntohs(hport));
>> +	DEBUG_ARG("laddr = %s", inet_ntoa((struct in_addr){.s_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
>>
> 


-- 
Alexey

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

[Samuel Thibault]

Alexey Kardashevskiy, le lun. 14 mai 2018 17:00:08 +1000, a ecrit:
> On 13/3/18 6:44 pm, Samuel Thibault wrote:
> > Alexey Kardashevskiy, on mar. 13 mars 2018 15:49:44 +1100, wrote:
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > 
> > Applied to my tree, thanks!
> 
> 
> And what is your tree, is this something to be merged sometime later
> somewhere? :)

When I manage to take the time to, yes.

Samuel

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

[Eric Blake]

On 05/14/2018 02:00 AM, Alexey Kardashevskiy wrote:
> On 13/3/18 6:44 pm, Samuel Thibault wrote:
>> Alexey Kardashevskiy, on mar. 13 mars 2018 15:49:44 +1100, wrote:
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> Applied to my tree, thanks!
> 
> 
> And what is your tree, is this something to be merged sometime later
> somewhere? :)

Per MAINTAINERS:

SLIRP
M: Samuel Thibault <samuel.thibault@ens-lyon.org>
M: Jan Kiszka <jan.kiszka@siemens.com>
S: Maintained
F: slirp/
F: net/slirp.c
F: include/net/slirp.h
T: git git://git.kiszka.org/qemu.git queues/slirp

which shows Jan's staging tree, but not Samuel's.

It's not a requirement for a maintainer to have a public-facing staging 
tree, but many of them do, as it gives you a chance to test that what 
will later be in a pull request matches what you expect.

At any rate, 
https://wiki.qemu.org/Contribute/SubmitAPatch#Is_my_patch_in.3F mentions 
that maintainers will batch together various related patches and send a 
pull request for inclusion in the main repository, thus even when a 
patch has been reviewed and staged, it may be another week or two before 
it lands in mainline.  Yes, it can feel slow, but in general it works 
out for the best (as we have more chances to flag potential problems 
before they affect everyone by being in mainline).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

[Alexey Kardashevskiy]

On 16/5/18 12:30 am, Eric Blake wrote:
> On 05/14/2018 02:00 AM, Alexey Kardashevskiy wrote:
>> On 13/3/18 6:44 pm, Samuel Thibault wrote:
>>> Alexey Kardashevskiy, on mar. 13 mars 2018 15:49:44 +1100, wrote:
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>
>>> Applied to my tree, thanks!
>>
>>
>> And what is your tree, is this something to be merged sometime later
>> somewhere? :)
> 
> Per MAINTAINERS:
> 
> SLIRP
> M: Samuel Thibault <samuel.thibault@ens-lyon.org>
> M: Jan Kiszka <jan.kiszka@siemens.com>
> S: Maintained
> F: slirp/
> F: net/slirp.c
> F: include/net/slirp.h
> T: git git://git.kiszka.org/qemu.git queues/slirp
> 
> which shows Jan's staging tree, but not Samuel's.
> 
> It's not a requirement for a maintainer to have a public-facing staging
> tree, but many of them do, as it gives you a chance to test that what will
> later be in a pull request matches what you expect.
> 
> At any rate,
> https://wiki.qemu.org/Contribute/SubmitAPatch#Is_my_patch_in.3F mentions
> that maintainers will batch together various related patches and send a
> pull request for inclusion in the main repository, thus even when a patch
> has been reviewed and staged, it may be another week or two before it lands
> in mainline.  Yes, it can feel slow, but in general it works out for the
> best (as we have more chances to flag potential problems before they affect
> everyone by being in mainline).


If it was 2 weeks or even 6 - I would not bother but when it is 2 months,
chances are it is forgotten/lost somewhere, hence my ping. I am not
complaining at all, just pinging :)



-- 
Alexey