Re: [Qemu-devel] [PATCH for-3.2 29/41] slirp: improve a bit the debug macros
Marc-André Lureau, le mer. 14 nov. 2018 16:36:31 +0400, a ecrit: > Let them accept multiple arguments. Simplify the inner argument > handling of DEBUG_ARGS/DEBUG_MISC_DEBUG_ERROR. > > Signed-off-by: Marc-André Lureau Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH for-3.2 29/41] slirp: improve a bit the debug macros
Hi
On Wed, Nov 14, 2018 at 6:04 PM Daniel P. Berrangé wrote:
>
> On Wed, Nov 14, 2018 at 04:36:31PM +0400, Marc-André Lureau wrote:
> > Let them accept multiple arguments. Simplify the inner argument
> > handling of DEBUG_ARGS/DEBUG_MISC_DEBUG_ERROR.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > slirp/debug.h | 47 --
> > slirp/arp_table.c | 12 ++--
> > slirp/bootp.c | 3 +--
> > slirp/cksum.c | 4 ++--
> > slirp/dhcpv6.c | 11 +--
> > slirp/ip6_icmp.c | 2 +-
> > slirp/ip_icmp.c| 18 +-
> > slirp/mbuf.c | 2 +-
> > slirp/ndp_table.c | 18 +-
> > slirp/slirp.c | 12 ++--
> > slirp/socket.c | 32 +++
> > slirp/tcp_input.c | 15 +++
> > slirp/tcp_output.c | 2 +-
> > slirp/tcp_subr.c | 4 ++--
> > slirp/udp.c| 6 +++---
> > slirp/udp6.c | 6 +++---
> > 16 files changed, 109 insertions(+), 85 deletions(-)
> >
> > diff --git a/slirp/debug.h b/slirp/debug.h
> > index 6cfa61edb3..ca3a4b04da 100644
> > --- a/slirp/debug.h
> > +++ b/slirp/debug.h
> > @@ -17,18 +17,45 @@
> >
> > extern int slirp_debug;
> >
> > -#define DEBUG_CALL(x) if (slirp_debug & DBG_CALL) { fprintf(dfd,
> > "%s...\n", x); fflush(dfd); }
> > -#define DEBUG_ARG(x, y) if (slirp_debug & DBG_CALL) { fputc(' ', dfd);
> > fprintf(dfd, x, y); fputc('\n', dfd); fflush(dfd); }
> > -#define DEBUG_ARGS(x) if (slirp_debug & DBG_CALL) { fprintf x ;
> > fflush(dfd); }
> > -#define DEBUG_MISC(x) if (slirp_debug & DBG_MISC) { fprintf x ;
> > fflush(dfd); }
> > -#define DEBUG_ERROR(x) if (slirp_debug & DBG_ERROR) {fprintf x ;
> > fflush(dfd); }
> > +#define DEBUG_CALL(fmt, ...) do { \
> > +if (slirp_debug & DBG_CALL) { \
> > +fprintf(dfd, fmt, ##__VA_ARGS__); \
> > +fprintf(dfd, "...\n"); \
> > +fflush(dfd);\
> > +} \
> > +} while (0)
> > +
> > +#define DEBUG_ARG(fmt, ...) do {\
> > +if (slirp_debug & DBG_CALL) { \
> > +fputc(' ', dfd);\
> > +fprintf(dfd, fmt, ##__VA_ARGS__); \
> > +fputc('\n', dfd); \
> > +fflush(dfd);\
> > +} \
> > +} while (0)
> > +
> > +#define DEBUG_ARGS(fmt, ...) DEBUG_ARG(fmt, ##__VA_ARGS__)
> > +
> > +#define DEBUG_MISC(fmt, ...) do { \
> > +if (slirp_debug & DBG_MISC) { \
> > +fprintf(dfd, fmt, ##__VA_ARGS__); \
> > +fflush(dfd);\
> > +} \
> > +} while (0)
> > +
> > +#define DEBUG_ERROR(fmt, ...) do { \
> > +if (slirp_debug & DBG_ERROR) { \
> > +fprintf(dfd, fmt, ##__VA_ARGS__); \
> > +fflush(dfd);\
> > +} \
> > +} while (0)
>
> I tend to think it would be nicer to change these to
> use g_debug, and #define G_LOG_DOMAIN "libslirp" in
> the .c files.
>
> This would allow apps to intercept the debug messages
> via a custom log handler.
Yes, even better would be structured logging, but it requires glib 2.50
One issue with the replacement is that we may want to keep the
slirp_debug filtering. If not, then we lose the classification.
Second issue is that slirp issues several debug calls and write \n in
the end. This is something you can't do with g_log. So it would need
some debug calls rewrite.
I would like to see this clean up patch applied before we decide how
to replace it with glog.
And yes, we should have a Slirp G_LOG_DOMAIN (I meant to do that, and forgot)
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH for-3.2 29/41] slirp: improve a bit the debug macros
On Wed, Nov 14, 2018 at 04:36:31PM +0400, Marc-André Lureau wrote:
> Let them accept multiple arguments. Simplify the inner argument
> handling of DEBUG_ARGS/DEBUG_MISC_DEBUG_ERROR.
>
> Signed-off-by: Marc-André Lureau
> ---
> slirp/debug.h | 47 --
> slirp/arp_table.c | 12 ++--
> slirp/bootp.c | 3 +--
> slirp/cksum.c | 4 ++--
> slirp/dhcpv6.c | 11 +--
> slirp/ip6_icmp.c | 2 +-
> slirp/ip_icmp.c| 18 +-
> slirp/mbuf.c | 2 +-
> slirp/ndp_table.c | 18 +-
> slirp/slirp.c | 12 ++--
> slirp/socket.c | 32 +++
> slirp/tcp_input.c | 15 +++
> slirp/tcp_output.c | 2 +-
> slirp/tcp_subr.c | 4 ++--
> slirp/udp.c| 6 +++---
> slirp/udp6.c | 6 +++---
> 16 files changed, 109 insertions(+), 85 deletions(-)
>
> diff --git a/slirp/debug.h b/slirp/debug.h
> index 6cfa61edb3..ca3a4b04da 100644
> --- a/slirp/debug.h
> +++ b/slirp/debug.h
> @@ -17,18 +17,45 @@
>
> extern int slirp_debug;
>
> -#define DEBUG_CALL(x) if (slirp_debug & DBG_CALL) { fprintf(dfd, "%s...\n",
> x); fflush(dfd); }
> -#define DEBUG_ARG(x, y) if (slirp_debug & DBG_CALL) { fputc(' ', dfd);
> fprintf(dfd, x, y); fputc('\n', dfd); fflush(dfd); }
> -#define DEBUG_ARGS(x) if (slirp_debug & DBG_CALL) { fprintf x ; fflush(dfd);
> }
> -#define DEBUG_MISC(x) if (slirp_debug & DBG_MISC) { fprintf x ; fflush(dfd);
> }
> -#define DEBUG_ERROR(x) if (slirp_debug & DBG_ERROR) {fprintf x ;
> fflush(dfd); }
> +#define DEBUG_CALL(fmt, ...) do { \
> +if (slirp_debug & DBG_CALL) { \
> +fprintf(dfd, fmt, ##__VA_ARGS__); \
> +fprintf(dfd, "...\n"); \
> +fflush(dfd);\
> +} \
> +} while (0)
> +
> +#define DEBUG_ARG(fmt, ...) do {\
> +if (slirp_debug & DBG_CALL) { \
> +fputc(' ', dfd);\
> +fprintf(dfd, fmt, ##__VA_ARGS__); \
> +fputc('\n', dfd); \
> +fflush(dfd);\
> +} \
> +} while (0)
> +
> +#define DEBUG_ARGS(fmt, ...) DEBUG_ARG(fmt, ##__VA_ARGS__)
> +
> +#define DEBUG_MISC(fmt, ...) do { \
> +if (slirp_debug & DBG_MISC) { \
> +fprintf(dfd, fmt, ##__VA_ARGS__); \
> +fflush(dfd);\
> +} \
> +} while (0)
> +
> +#define DEBUG_ERROR(fmt, ...) do { \
> +if (slirp_debug & DBG_ERROR) { \
> +fprintf(dfd, fmt, ##__VA_ARGS__); \
> +fflush(dfd);\
> +} \
> +} while (0)
I tend to think it would be nicer to change these to
use g_debug, and #define G_LOG_DOMAIN "libslirp" in
the .c files.
This would allow apps to intercept the debug messages
via a custom log handler.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PATCH for-3.2 29/41] slirp: improve a bit the debug macros
Let them accept multiple arguments. Simplify the inner argument
handling of DEBUG_ARGS/DEBUG_MISC_DEBUG_ERROR.
Signed-off-by: Marc-André Lureau
---
slirp/debug.h | 47 --
slirp/arp_table.c | 12 ++--
slirp/bootp.c | 3 +--
slirp/cksum.c | 4 ++--
slirp/dhcpv6.c | 11 +--
slirp/ip6_icmp.c | 2 +-
slirp/ip_icmp.c| 18 +-
slirp/mbuf.c | 2 +-
slirp/ndp_table.c | 18 +-
slirp/slirp.c | 12 ++--
slirp/socket.c | 32 +++
slirp/tcp_input.c | 15 +++
slirp/tcp_output.c | 2 +-
slirp/tcp_subr.c | 4 ++--
slirp/udp.c| 6 +++---
slirp/udp6.c | 6 +++---
16 files changed, 109 insertions(+), 85 deletions(-)
diff --git a/slirp/debug.h b/slirp/debug.h
index 6cfa61edb3..ca3a4b04da 100644
--- a/slirp/debug.h
+++ b/slirp/debug.h
@@ -17,18 +17,45 @@
extern int slirp_debug;
-#define DEBUG_CALL(x) if (slirp_debug & DBG_CALL) { fprintf(dfd, "%s...\n",
x); fflush(dfd); }
-#define DEBUG_ARG(x, y) if (slirp_debug & DBG_CALL) { fputc(' ', dfd);
fprintf(dfd, x, y); fputc('\n', dfd); fflush(dfd); }
-#define DEBUG_ARGS(x) if (slirp_debug & DBG_CALL) { fprintf x ; fflush(dfd); }
-#define DEBUG_MISC(x) if (slirp_debug & DBG_MISC) { fprintf x ; fflush(dfd); }
-#define DEBUG_ERROR(x) if (slirp_debug & DBG_ERROR) {fprintf x ; fflush(dfd); }
+#define DEBUG_CALL(fmt, ...) do { \
+if (slirp_debug & DBG_CALL) { \
+fprintf(dfd, fmt, ##__VA_ARGS__); \
+fprintf(dfd, "...\n"); \
+fflush(dfd);\
+} \
+} while (0)
+
+#define DEBUG_ARG(fmt, ...) do {\
+if (slirp_debug & DBG_CALL) { \
+fputc(' ', dfd);\
+fprintf(dfd, fmt, ##__VA_ARGS__); \
+fputc('\n', dfd); \
+fflush(dfd);\
+} \
+} while (0)
+
+#define DEBUG_ARGS(fmt, ...) DEBUG_ARG(fmt, ##__VA_ARGS__)
+
+#define DEBUG_MISC(fmt, ...) do { \
+if (slirp_debug & DBG_MISC) { \
+fprintf(dfd, fmt, ##__VA_ARGS__); \
+fflush(dfd);\
+} \
+} while (0)
+
+#define DEBUG_ERROR(fmt, ...) do { \
+if (slirp_debug & DBG_ERROR) { \
+fprintf(dfd, fmt, ##__VA_ARGS__); \
+fflush(dfd);\
+} \
+} while (0)
#else
-#define DEBUG_CALL(x)
-#define DEBUG_ARG(x, y)
-#define DEBUG_ARGS(x)
-#define DEBUG_MISC(x)
-#define DEBUG_ERROR(x)
+#define DEBUG_CALL(fmt, ...)
+#define DEBUG_ARG(fmt, ...)
+#define DEBUG_ARGS(fmt, ...)
+#define DEBUG_MISC(fmt, ...)
+#define DEBUG_ERROR(fmt, ...)
#endif
diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index f81963bb88..ce19e6e7c0 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -34,9 +34,9 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t
ethaddr[ETH_ALEN])
DEBUG_CALL("arp_table_add");
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]));
+DEBUG_ARGS(" hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
+ ethaddr[0], ethaddr[1], ethaddr[2],
+ ethaddr[3], ethaddr[4], ethaddr[5]);
if (ip_addr == 0 || ip_addr == 0x || ip_addr == broadcast_addr) {
/* Do not register broadcast addresses */
@@ -79,9 +79,9 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
for (i = 0; i < ARP_TABLE_SIZE; i++) {
if (arptbl->table[i].ar_sip == ip_addr) {
memcpy(out_ethaddr, arptbl->table[i].ar_sha, ETH_ALEN);
-DEBUG_ARGS((dfd, " found hw addr =
%02x:%02x:%02x:%02x:%02x:%02x\n",
-out_ethaddr[0], out_ethaddr[1], out_ethaddr[2],
-out_ethaddr[3], out_ethaddr[4], out_ethaddr[5]));
+DEBUG_ARGS(" found hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
+ out_ethaddr[0], out_ethaddr[1], out_ethaddr[2],
+ out_ethaddr[3], out_ethaddr[4], out_ethaddr[5]);
return 1;
}
}
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 7b1af73c95..5ab6692038 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -37,8 +37,7 @@
static const uint8_t rfc1533_cookie[] = { RFC1533_COOKIE };
#ifdef DEBUG
-#define DPRINTF(fmt, ...) \
-do if (slirp_debug & DBG_CALL) { fprintf(dfd, fmt, ## __VA_ARGS__);
fflush(dfd); } while (0)
+#define DPRINTF(fmt, ...) DEBUG_CALL(fmt, ##__VA_ARGS__)
#else
#define
