On Fri, Feb 26, 2021 at 4:06 AM <marcandre.lur...@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lur...@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-triv...@nongnu.org
> Signed-off-by: Marc-André Lureau <marcandre.lur...@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?
