From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6ADBBA04DE; Fri, 30 Oct 2020 15:59:23 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7E99AC99C; Fri, 30 Oct 2020 15:59:20 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by dpdk.org (Postfix) with ESMTP id 7EAC6C96C for ; Fri, 30 Oct 2020 15:59:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1604069958; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=OX62y5QYPCfFjfTi/I5IV6+M9czMGc13J+NcCJhoLfk=; b=X+kTRIBAF3EvrDQXM/WOPvhxcFWwpEkcVZjg2WWEl0nNieGQezgORgCM0pp8CIpykGh4T9 U1sCUU02GwPZylu2awh3xDqV8RWmOyXC9h0/s7mu9s7gAOap8gZaZ5q/jZeaZ1fqRGF2OI LGdao0qg2pkW7/TMzS2X8Dixk3TkcZA= Received: from mail-ua1-f72.google.com (mail-ua1-f72.google.com [209.85.222.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-44-0YjW7SdFNC6heF1NTVjWSQ-1; Fri, 30 Oct 2020 10:59:14 -0400 X-MC-Unique: 0YjW7SdFNC6heF1NTVjWSQ-1 Received: by mail-ua1-f72.google.com with SMTP id o3so808201uaw.0 for ; Fri, 30 Oct 2020 07:59:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OX62y5QYPCfFjfTi/I5IV6+M9czMGc13J+NcCJhoLfk=; b=q2N70n/kHUwCXSwqja445spc5ncOjgt5U7V+/V7PDs5WiH6/1AG3noTAT8TbBYCcw+ Ska0kl8Ea/rEQclh+oyD22GC7kE4c4X2g8h0c6ZTDBhmg9ZJ+jq6qOvHyTbMgCkJjsxa 13BsAr8e9REDbB3ar4l+poaREMt2myhAWoGqkdtL0xrqARZUklyOpKe24oTjuKjce4fj VDg6I1+5BGRFEQR8t1YCqpiEXuE76A8E3XLwD9MJt3fc+J2rIXAMbuFj8c6oD/Ruv2xG RSbtUc0SOWZ8jZCbofJAjBXUm86opk+G65FYiQ9CR1YArZJG1cbN6K8KQypuRvcasBVm TIkA== X-Gm-Message-State: AOAM531kNPVcG0dljGoxQuLoP8gOQWRTrVS7I8AudTUsITkGMs5Gaxhw Lddn8QGbAZvjSMHGzB0kDtrk15KPN+NNyXvN6Lyd1I7WyvbQOB5aEp9t58TmJnCdgsbfdDQUn8H M0BJVlrJ7vONDLgblpvU= X-Received: by 2002:a67:d78b:: with SMTP id q11mr7246433vsj.17.1604069953584; Fri, 30 Oct 2020 07:59:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy79yFzj+pPt/hj4X6T6wKsZ8AZMhieEpNs+eJjSlKCZsoiKq1jZqMlESTeXzaU9PdcfIP4xGzr6NtgM6yqsy8= X-Received: by 2002:a67:d78b:: with SMTP id q11mr7246407vsj.17.1604069953343; Fri, 30 Oct 2020 07:59:13 -0700 (PDT) MIME-Version: 1.0 References: <20200731034520.30791-1-gaurav1086@gmail.com> <20200808163906.8021-1-gaurav1086@gmail.com> In-Reply-To: From: David Marchand Date: Fri, 30 Oct 2020 15:59:02 +0100 Message-ID: To: Gaurav Singh , "Power, Ciara" Cc: Honnappa Nagarahalli , "dev@dpdk.org" , nd Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] lib/metrics: fix memory leak X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hello Gaurav, On Tue, Sep 22, 2020 at 12:27 PM Power, Ciara wrote: > >-----Original Message----- > >From: dev On Behalf Of Honnappa Nagarahalli > >Sent: Tuesday 11 August 2020 22:01 > >To: Gaurav Singh ; dev@dpdk.org > >Cc: nd ; Honnappa Nagarahalli > >; nd > >Subject: Re: [dpdk-dev] [PATCH] lib/metrics: fix memory leak > > > > > > > >> > >> fix memory leak > >> > >> Fixes: c5b7197f66 ("telemetry: move some functions to metrics > >> library") > >> > >> Signed-off-by: Gaurav Singh > >> --- > >> lib/librte_metrics/rte_metrics_telemetry.c | 21 ++++++++++++++++----- > >> 1 file changed, 16 insertions(+), 5 deletions(-) > > > I think this commit message should be more descriptive, and is missing Cc: stable@dpdk.org > > > >> diff --git a/lib/librte_metrics/rte_metrics_telemetry.c > >> b/lib/librte_metrics/rte_metrics_telemetry.c > >> index 289ebae0b..7b6d1063c 100644 > >> --- a/lib/librte_metrics/rte_metrics_telemetry.c > >> +++ b/lib/librte_metrics/rte_metrics_telemetry.c > >> @@ -41,12 +41,17 @@ > >> rte_metrics_tel_reg_port_ethdev_to_metrics(uint16_t > >> port_id) > >> } > >> > >> xstats_names = malloc(sizeof(*xstats_names) * num_xstats); > >> + if (xstats_names == NULL) { > >> + METRICS_LOG_ERR("Failed to malloc memory for > >> xstats_names"); > >> + return -ENOMEM; > >> + } > >> + > >> eth_xstats_names = malloc(sizeof(struct rte_eth_xstat_name) > >> * num_xstats); > >> - if (eth_xstats_names == NULL || xstats_names == NULL) { > >> - METRICS_LOG_ERR("Failed to malloc memory for > >> xstats_names"); > >> - ret = -ENOMEM; > >> - goto free_xstats; > >> + if (eth_xstats_names == NULL) { > >> + METRICS_LOG_ERR("Failed to malloc memory for > >> eth_xstats_names"); > >> + free(xstats_names); > >> + return -ENOMEM; > >> } > > Is there a reason for the above changes? I think they are unrelated to > the memory leak this patch is fixing. > > >> if (rte_eth_xstats_get_names(port_id, > >> @@ -167,9 +172,15 @@ rte_metrics_tel_format_port(uint32_t pid, json_t > >> *ports, > >> } > >> > >> metrics = malloc(sizeof(struct rte_metric_value) * num_metrics); > >> + if (metrics == NULL) { > >> + METRICS_LOG_ERR("Cannot allocate memory"); > >> + return -ENOMEM; > >> + } > >> + > >> names = malloc(sizeof(struct rte_metric_name) * num_metrics); > >> - if (metrics == NULL || names == NULL) { > >> + if (names == NULL) { > >> METRICS_LOG_ERR("Cannot allocate memory"); > >> + free(metrics); > >> return -ENOMEM; > >> } > > This does fix the resource leak, but I do think it can be done in a > simpler way, as shown in the patch I sent to fix the logged coverity issue > for this: https://patchwork.dpdk.org/patch/78052/ > I will remove my patch seeing as this patch is fixing the same thing. I agree with Ciara. Could you respin? Thanks. -- David Marchand