[ovs-dev] dpdk: Remove unneeded log message copy.
diff mbox series

Message ID 1567769162-10110-1-git-send-email-david.marchand@redhat.com
State New
Headers show
Series
  • [ovs-dev] dpdk: Remove unneeded log message copy.
Related show

Commit Message

David Marchand Sept. 6, 2019, 11:26 a.m. UTC
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(-)

Comments

Eelco Chaudron Sept. 6, 2019, 1:18 p.m. UTC | #1
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>
Ilya Maximets Sept. 6, 2019, 1:24 p.m. UTC | #2
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;
>  }
>  
>
David Marchand Sept. 6, 2019, 2:18 p.m. UTC | #3
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!

Patch
diff mbox series

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;
 }