slirp/debug: Print IP addresses in human readable form

PATCH Superseded Alexey Kardashevskiy 5 participants 7 messages Feb 1, 2018 – Mar 7, 2018

lore

Share & tools

lore

Alexey Kardashevskiy

author

2018-02-01 09:36 UTC #0 lore raw

PATCH 1/1 slirp/debug: Print IP addresses in human readable form

Signed-off-by Alexey Kardashevskiy <[email protected]>

---
 slirp/arp_table.c | 4 ++--
 slirp/socket.c    | 8 ++++----
 slirp/udp.c       | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

slirp/arp_table.c +2 -2

index 3547043..bac608f 100644
@@ -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) {

slirp/socket.c +4 -4

index cb7b5b6..61347d1 100644
@@ -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);

slirp/udp.c +2 -2

index 227d779..e5bf065 100644
@@ -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

Signature · 1 line

2.11.0

[email protected]

bot

2018-02-01 10:01 UTC #1 lore raw

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

Hi,

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

Type: series
Message-id: [email protected]
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 [email protected]

Alexey Kardashevskiy

author

2018-03-07 03:38 UTC #2 lore raw

In reply to Alexey Kardashevskiy

Quoted 1 line · Alexey Kardashevskiy

Signed-off-by Alexey Kardashevskiy <[email protected]>

Ping?

Quoted 63 lines

---
 slirp/arp_table.c | 4 ++--
 slirp/socket.c    | 8 ++++----
 slirp/udp.c       | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

slirp/arp_table.c +2 -2

index 3547043..bac608f 100644
@@ -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) {

slirp/socket.c +4 -4

index cb7b5b6..61347d1 100644
@@ -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);

slirp/udp.c +2 -2

index 227d779..e5bf065 100644
@@ -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

Signature · 1 line

Alexey

Thomas Huth

2018-03-07 06:24 UTC #3 lore raw

Quoted 19 lines · Alexey Kardashevskiy

Quoted 1 line · Alexey Kardashevskiy
Signed-off-by Alexey Kardashevskiy <[email protected]>
Ping?
Quoted 13 lines
---
 slirp/arp_table.c | 4 ++--
 slirp/socket.c    | 8 ++++----
 slirp/udp.c       | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)
slirp/arp_table.c +0 -0
index 3547043..bac608f 100644
@@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN])
     int i;
 

Quoted 3 lines

slirp/arp_table.c void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN]) · line 36
Quoted 3 lines
slirp/arp_table.c +1 -1
     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

Alexey Kardashevskiy

author

2018-03-07 06:32 UTC #4 lore raw

Quoted 20 lines · Thomas Huth

Quoted 19 lines · Alexey Kardashevskiy
Quoted 1 line · Alexey Kardashevskiy
Signed-off-by Alexey Kardashevskiy <[email protected]>
Ping?
Quoted 13 lines
---
 slirp/arp_table.c | 4 ++--
 slirp/socket.c    | 8 ++++----
 slirp/udp.c       | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)
slirp/arp_table.c +0 -0
index 3547043..bac608f 100644
@@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN])
     int i;
 

Quoted 7 lines

Quoted 3 lines
slirp/arp_table.c void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN]) · line 36
Quoted 3 lines
slirp/arp_table.c +1 -1
     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 :)

Signature · 1 line

Alexey

Samuel Thibault

2018-03-07 22:39 UTC #5 lore raw

In reply to Thomas Huth

Hello,

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

Quoted 10 lines · Thomas Huth, on mer. 07 mars 2018 07:24:16 +0100, wrote:

Quoted 10 lines
slirp/arp_table.c void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN]) · line 36
Quoted 10 lines
slirp/arp_table.c +1 -1
index 3547043..bac608f 100644
@@ -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

Paolo Bonzini

2018-03-07 14:47 UTC #6 lore raw

In reply to Alexey Kardashevskiy

Quoted 6 lines · Alexey Kardashevskiy

Signed-off-by Alexey Kardashevskiy <[email protected]>

---
 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

Quoted 57 lines

slirp/arp_table.c +2 -2

index 3547043..bac608f 100644
@@ -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) {

slirp/socket.c +4 -4

index cb7b5b6..61347d1 100644
@@ -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);

slirp/udp.c +2 -2

index 227d779..e5bf065 100644
@@ -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