| Message ID | 20210226120607.675753-1-marcandre.lureau@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: remove qemu_ether_ntoa() | expand |
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?
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