From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 42B7DA0527;
	Mon,  9 Nov 2020 14:36:59 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 26A105AB8;
	Mon,  9 Nov 2020 14:36:58 +0100 (CET)
Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231])
 by dpdk.org (Postfix) with ESMTP id 80D1F5AA7
 for <dev@dpdk.org>; Mon,  9 Nov 2020 14:36:55 +0100 (CET)
Received: from u256.net (lfbn-poi-1-1343-16.w90-78.abo.wanadoo.fr [90.78.4.16])
 (Authenticated sender: grive@u256.net)
 by relay11.mail.gandi.net (Postfix) with ESMTPSA id 833F310000B;
 Mon,  9 Nov 2020 13:36:54 +0000 (UTC)
Date: Mon, 9 Nov 2020 14:36:49 +0100
From: =?utf-8?Q?Ga=C3=ABtan?= Rivet <grive@u256.net>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, Hemant Agrawal <hemant.agrawal@nxp.com>,
 Sachin Saxena <sachin.saxena@oss.nxp.com>
Message-ID: <20201109130859.ixz6pj57wwzfve3s@u256.net>
References: <20201109113147.962882-1-grive@u256.net>
 <CAJFAV8xtCCjneYkhJ8q6m=2hRKf+a590tPTDZj8sf3r6zCev1w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJFAV8xtCCjneYkhJ8q6m=2hRKf+a590tPTDZj8sf3r6zCev1w@mail.gmail.com>
Subject: Re: [dpdk-dev] [PATCH] bus/dpaa: optimize device name parsing
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 09/11/20 12:58 +0100, David Marchand wrote:
> On Mon, Nov 9, 2020 at 12:32 PM Gaetan Rivet <grive@u256.net> wrote:
> > +       delta = 0;
> > +       if (strncmp(name, "dpaa_bus:", 9) == 0) {
> > +               delta = 9;
> > +       } else if (strncmp(name, "name=", 5) == 0) {
> > +               delta = 5;
> > +       }
> > +
> > +       if (sscanf(&name[delta], "fm%u-mac%u", &i, &j) == 2) {
> > +               if (i >= 2 || j >= 16)
> > +                       return -EINVAL;
> > +               if (out != NULL) {
> > +                       char *out_name = out;
> > +
> > +                       if (rte_strscpy(out_name, &name[delta], 10) < 0)
> > +                               return -ENAMETOOLONG;
> > +                       /* Because 'i' can only be 0 or 1, fm%u is fixed size ;
> > +                        * mac%u needs to be checked for optional end ','.
> > +                        */
> > +                       if (out_name[9] == ',')
> > +                               out_name[9] = '\0';
> >                 }
> > +               return 0;
> > +       } else {
> > +               return -EINVAL;
> >         }
> 
> Mmm, we always return from each of the previous branches?
> Then the ENODEV return after is dead code.
> 
> Reading again the diff, you could probably return at the sscanf step.
> if (sscanf(&name[delta], "fm%u-mac%u", &i, &j) != 2 || i >= 2 || j >= 16)
>         return -EINVAL;
> 
> 

Yes good point, I will refactor.

> >
> > -       free(dup_name);
> > -       return -EINVAL;
> > +       return -ENODEV;
> >  }
> >
> >  #define DPAA_DEV_PATH1 "/sys/devices/platform/soc/soc:fsl,dpaa"
> 
> 
> -- 
> David Marchand
> 

-- 
Gaëtan