qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] net: remove qemu_ether_ntoa()


From: Doug Evans
Subject: Re: [PATCH] net: remove qemu_ether_ntoa()
Date: Fri, 26 Feb 2021 08:21:21 -0800

On Fri, Feb 26, 2021 at 4:06 AM <marcandre.lureau@redhat.com> wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>

The function is not thread-safe and sets a bad example. It's used in a
single place for tracing, so open-code the format string like other
trace events with MAC addresses.

Cc: qemu-trivial@nongnu.org
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu-common.h |  1 -
 net/announce.c        |  8 +++++++-
 util/cutils.c         | 13 -------------
 net/trace-events      |  2 +-
 4 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 654621444e..209133bfca 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -132,7 +132,6 @@ void qemu_hexdump(FILE *fp, const char *prefix,
  */
 int parse_debug_env(const char *name, int max, int initial);

-const char *qemu_ether_ntoa(const MACAddr *mac);
 void page_size_init(void);

 /* returns non-zero if dump is in progress, otherwise zero is
diff --git a/net/announce.c b/net/announce.c
index 26f057f5ee..fc0c6baace 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -146,7 +146,13 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)

     trace_qemu_announce_self_iter(timer->params.has_id ? timer->params.id : "_",
                                   nic->ncs->name,
-                                  qemu_ether_ntoa(&nic->conf->macaddr), skip);
+                                  nic->conf->macaddr.a[0],
+                                  nic->conf->macaddr.a[1],
+                                  nic->conf->macaddr.a[2],
+                                  nic->conf->macaddr.a[3],
+                                  nic->conf->macaddr.a[4],
+                                  nic->conf->macaddr.a[5],
+                                  skip);

     if (!skip) {
         len = announce_self_create(buf, nic->conf->macaddr.a);
diff --git a/util/cutils.c b/util/cutils.c
index 70c7d6efbd..b5460a72b4 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -847,19 +847,6 @@ int parse_debug_env(const char *name, int max, int initial)
     return debug;
 }

-/*
- * Helper to print ethernet mac address
- */
-const char *qemu_ether_ntoa(const MACAddr *mac)
-{
-    static char ret[18];
-
-    snprintf(ret, sizeof(ret), "%02x:%02x:%02x:%02x:%02x:%02x",
-             mac->a[0], mac->a[1], mac->a[2], mac->a[3], mac->a[4], mac->a[5]);
-
-    return ret;
-}
-
 /*
  * Return human readable string for size @val.
  * @val can be anything that uint64_t allows (no more than "16 EiB").
diff --git a/net/trace-events b/net/trace-events
index bfaff7891d..07d6203602 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -1,7 +1,7 @@
 # See docs/devel/tracing.txt for syntax documentation.

 # announce.c
-qemu_announce_self_iter(const char *id, const char *name, const char *mac, int skip) "%s:%s:%s skip: %d"
+qemu_announce_self_iter(const char *id, const char *name, char mac0, char mac1, char mac2, char mac3, char mac4, char mac5, int skip) "%s:%s:%02x:%02x:%02x:%02x:%02x:%02x skip: %d"
 qemu_announce_timer_del(bool free_named, bool free_timer, char *id) "free named: %d free timer: %d id: %s"

 # vhost-user.c
--
2.29.0


It's pretty tedious to open code that.
How about instead change qemu_ether_ntoa to being thread-safe?

And, separately, add a bit of type safety and put eth addrs in a struct?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]