include/qemu-common.h | 1 - net/announce.c | 8 +++++++- util/cutils.c | 13 ------------- net/trace-events | 2 +- 4 files changed, 8 insertions(+), 16 deletions(-)
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
On 2/26/21 1:06 PM, 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(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
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?
© 2016 - 2025 Red Hat, Inc.