From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id BEB00A0544; Fri, 23 Sep 2022 08:35:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 65173400D7; Fri, 23 Sep 2022 08:35:33 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id A7BDD4003C for ; Fri, 23 Sep 2022 08:35:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663914931; 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=+3Y5hV2nXXUMopdLe1MvYAbuSjhyRn2O4LjJU1f1jFo=; b=dMlurhU1OpN3HHwOpw4AcJtV7CbXssKPzmgP3/JxG/x274LLTX/Ys8M00d3QPtepa4cFxv v2dfV5AZ2x2J5q+uwiDoF3mEBF8eHqKdVHNK9u1EbAM/Za13uny1tE+8Wt48fPZ6ZCZ/7B dL1GwSOsnmDyASFbwOSKHc7Ccx3RjqI= Received: from mail-lj1-f200.google.com (mail-lj1-f200.google.com [209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-541-PcD82O9zOaSGgdPTGAnepQ-1; Fri, 23 Sep 2022 02:35:29 -0400 X-MC-Unique: PcD82O9zOaSGgdPTGAnepQ-1 Received: by mail-lj1-f200.google.com with SMTP id bx10-20020a05651c198a00b0026c1cdb5b4cso3602313ljb.2 for ; Thu, 22 Sep 2022 23:35:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=+3Y5hV2nXXUMopdLe1MvYAbuSjhyRn2O4LjJU1f1jFo=; b=N1J/HMaGiAh41PhKZdbZQyaRK+FBWBp5bnFmUqxX9uUHQLSdt0N2jQOyqDag3Abm5M WIiqYlr5J+uyyyWvsbbgOjMZas+FeT/VfqU9VQMAI1CG6E/Yl2HQYRnKrvj25qLjozam gJseBDJ5MnRPkwONVgH2kfAkN+Ysb9i/Popmdvj/ix4rNyR/6AtcmgELvnHrX/WNBF/y AhfGpV0J74te22cyIP9ZKF4LOh8UWX3+fIwA/jhqjPNTYekzJGSQl9Vj49kLpe8+X3jh L5UKSrV/lVjltSicSy+ZHFekvISQewnc8zCUDJfi5S3pzPFmvbxTNLmm1/svaUPF2sE8 259w== X-Gm-Message-State: ACrzQf3+lfevFvDy9oqgdB7xxeoc9xjUqJgSyBewBzvZmhv6lthDC8Kr 6uyCvufIZ93ntlihwFTBNpsPPC4np5ubO+XyfFf8h+FGoN9tGuiPDxi4KzFliGmvCKsIdhgZm6S 4N4X5ad2xabyZEBFnp3U= X-Received: by 2002:a05:6512:1289:b0:49f:1b3d:88c5 with SMTP id u9-20020a056512128900b0049f1b3d88c5mr2644769lfs.499.1663914928390; Thu, 22 Sep 2022 23:35:28 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5H/5Rj+93WXKdanVUv12ELubQ5G1I3VKoBcI0XP9jGDwFfCnso6V0qBG11VqWMB3cr3EYfrWTmLrkN79pgzHY= X-Received: by 2002:a05:6512:1289:b0:49f:1b3d:88c5 with SMTP id u9-20020a056512128900b0049f1b3d88c5mr2644764lfs.499.1663914928146; Thu, 22 Sep 2022 23:35:28 -0700 (PDT) MIME-Version: 1.0 References: <20220921120359.2201131-1-david.marchand@redhat.com> <20220921120359.2201131-4-david.marchand@redhat.com> In-Reply-To: From: David Marchand Date: Fri, 23 Sep 2022 08:35:17 +0200 Message-ID: Subject: Re: [EXT] [PATCH 3/8] trace: fix leak with regexp To: Sunil Kumar Kori Cc: "dev@dpdk.org" , "stable@dpdk.org" , Jerin Jacob Kollanukkaran X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Sep 22, 2022 at 1:00 PM Sunil Kumar Kori wrote: > > @@ -210,15 +210,18 @@ rte_trace_regexp(const char *regex, bool enable) > > return -EINVAL; > > > > STAILQ_FOREACH(tp, &tp_list, next) { > > - if (regexec(&r, tp->name, 0, NULL, 0) == 0) { > > - if (enable) > > - rc = rte_trace_point_enable(tp->handle); > > - else > > - rc = rte_trace_point_disable(tp->handle); > > - found = 1; > > + if (regexec(&r, tp->name, 0, NULL, 0) != 0) > > + continue; > > + > > + if (enable) > > + rc = rte_trace_point_enable(tp->handle); > > + else > > + rc = rte_trace_point_disable(tp->handle); > > + if (rc < 0) { > > + found = 0; > > + break; > > } > > - if (rc < 0) > > - return rc; > > + found = 1; > > } > > regfree(&r); > > > > -- > > I understand the problem addressed by this fix but may be following changes will be sufficient to fix it. > Please highlight, If I am missing. Just trying to reduce the line of changes. > > @@ -220,8 +220,10 @@ rte_trace_regexp(const char *regex, bool enable) > rc = rte_trace_point_disable(tp->handle); > found = 1; > } > - if (rc < 0) > - return rc; > + if (rc < 0) { > + found = 0; > + break; > + } > } rc is compared against 0 for all non-matching tracepoints. This is unnecessary and fragile. I can split this change in two: one for the fix, and one other where the loop is updated with a continue and an inverted matching condition like above. If going like this, I would update rte_trace_pattern() too, in the second patch. WDYT? -- David Marchand