| Message ID | 1567769162-10110-1-git-send-email-david.marchand@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] dpdk: Remove unneeded log message copy. | expand |
On 6 Sep 2019, at 13:26, David Marchand wrote: > No need to duplicate and null-terminate the passed buffer. > We can directly give it to the vlog subsystem using a dynamic precision > in the format string. > > Signed-off-by: David Marchand <david.marchand@redhat.com> Change looks good to me! Acked-by: Eelco Chaudron <echaudro@redhat.com>
On 06.09.2019 14:26, David Marchand wrote: > No need to duplicate and null-terminate the passed buffer. > We can directly give it to the vlog subsystem using a dynamic precision > in the format string. > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- Thanks for a patch. Looks good at a first glance. One small nit is that you need a space between "(int)" and "size", but this could be, probably, fixed while applying. Best regards, Ilya Maximets. > lib/dpdk.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/lib/dpdk.c b/lib/dpdk.c > index f31e158..481a109 100644 > --- a/lib/dpdk.c > +++ b/lib/dpdk.c > @@ -232,34 +232,32 @@ construct_dpdk_args(const struct smap *ovs_other_config, struct svec *args) > static ssize_t > dpdk_log_write(void *c OVS_UNUSED, const char *buf, size_t size) > { > - char *str = xmemdup0(buf, size); > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(600, 600); > static struct vlog_rate_limit dbg_rl = VLOG_RATE_LIMIT_INIT(600, 600); > > switch (rte_log_cur_msg_loglevel()) { > case RTE_LOG_DEBUG: > - VLOG_DBG_RL(&dbg_rl, "%s", str); > + VLOG_DBG_RL(&dbg_rl, "%.*s", (int)size, buf); > break; > case RTE_LOG_INFO: > case RTE_LOG_NOTICE: > - VLOG_INFO_RL(&rl, "%s", str); > + VLOG_INFO_RL(&rl, "%.*s", (int)size, buf); > break; > case RTE_LOG_WARNING: > - VLOG_WARN_RL(&rl, "%s", str); > + VLOG_WARN_RL(&rl, "%.*s", (int)size, buf); > break; > case RTE_LOG_ERR: > - VLOG_ERR_RL(&rl, "%s", str); > + VLOG_ERR_RL(&rl, "%.*s", (int)size, buf); > break; > case RTE_LOG_CRIT: > case RTE_LOG_ALERT: > case RTE_LOG_EMERG: > - VLOG_EMER("%s", str); > + VLOG_EMER("%.*s", (int)size, buf); > break; > default: > OVS_NOT_REACHED(); > } > > - free(str); > return size; > } > >
On Fri, Sep 6, 2019 at 3:24 PM Ilya Maximets <i.maximets@samsung.com> wrote: > > On 06.09.2019 14:26, David Marchand wrote: > > No need to duplicate and null-terminate the passed buffer. > > We can directly give it to the vlog subsystem using a dynamic precision > > in the format string. > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > --- > > Thanks for a patch. Looks good at a first glance. > > One small nit is that you need a space between "(int)" and "size", but > this could be, probably, fixed while applying. Tell me, I don't mind sending a v2. Thanks!
On 9/6/2019 3:18 PM, David Marchand wrote: > On Fri, Sep 6, 2019 at 3:24 PM Ilya Maximets <i.maximets@samsung.com> wrote: >> >> On 06.09.2019 14:26, David Marchand wrote: >>> No need to duplicate and null-terminate the passed buffer. >>> We can directly give it to the vlog subsystem using a dynamic precision >>> in the format string. >>> >>> Signed-off-by: David Marchand <david.marchand@redhat.com> >>> --- >> >> Thanks for a patch. Looks good at a first glance. >> >> One small nit is that you need a space between "(int)" and "size", but >> this could be, probably, fixed while applying. > > Tell me, I don't mind sending a v2. > Thanks! > No need for the v2, can apply on commit. Thanks Ian
On 9/25/2019 6:30 PM, Stokes, Ian wrote: > > > On 9/6/2019 3:18 PM, David Marchand wrote: >> On Fri, Sep 6, 2019 at 3:24 PM Ilya Maximets <i.maximets@samsung.com> >> wrote: >>> >>> On 06.09.2019 14:26, David Marchand wrote: >>>> No need to duplicate and null-terminate the passed buffer. >>>> We can directly give it to the vlog subsystem using a dynamic precision >>>> in the format string. >>>> >>>> Signed-off-by: David Marchand <david.marchand@redhat.com> >>>> --- >>> >>> Thanks for a patch. Looks good at a first glance. >>> >>> One small nit is that you need a space between "(int)" and "size", but >>> this could be, probably, fixed while applying. >> >> Tell me, I don't mind sending a v2. >> Thanks! >> > > No need for the v2, can apply on commit. > Just pushed to master. Thanks Ian > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/dpdk.c b/lib/dpdk.c index f31e158..481a109 100644 --- a/lib/dpdk.c +++ b/lib/dpdk.c @@ -232,34 +232,32 @@ construct_dpdk_args(const struct smap *ovs_other_config, struct svec *args) static ssize_t dpdk_log_write(void *c OVS_UNUSED, const char *buf, size_t size) { - char *str = xmemdup0(buf, size); static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(600, 600); static struct vlog_rate_limit dbg_rl = VLOG_RATE_LIMIT_INIT(600, 600); switch (rte_log_cur_msg_loglevel()) { case RTE_LOG_DEBUG: - VLOG_DBG_RL(&dbg_rl, "%s", str); + VLOG_DBG_RL(&dbg_rl, "%.*s", (int)size, buf); break; case RTE_LOG_INFO: case RTE_LOG_NOTICE: - VLOG_INFO_RL(&rl, "%s", str); + VLOG_INFO_RL(&rl, "%.*s", (int)size, buf); break; case RTE_LOG_WARNING: - VLOG_WARN_RL(&rl, "%s", str); + VLOG_WARN_RL(&rl, "%.*s", (int)size, buf); break; case RTE_LOG_ERR: - VLOG_ERR_RL(&rl, "%s", str); + VLOG_ERR_RL(&rl, "%.*s", (int)size, buf); break; case RTE_LOG_CRIT: case RTE_LOG_ALERT: case RTE_LOG_EMERG: - VLOG_EMER("%s", str); + VLOG_EMER("%.*s", (int)size, buf); break; default: OVS_NOT_REACHED(); } - free(str); return size; }
No need to duplicate and null-terminate the passed buffer. We can directly give it to the vlog subsystem using a dynamic precision in the format string. Signed-off-by: David Marchand <david.marchand@redhat.com> --- lib/dpdk.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)