Hey,
Sorry for my late response, I sent a fix (v7):
https://patches.dpdk.org/project/dpdk/patch/20250212163836.178976-2-shper...@nvidia.com/
I added another parameter to the parse function - the size of the memory
pointed by addr.
so the function signature now is:
int XXX_parse(const char *name, void *addr, int addr_size, int *out_size)
So I now use it in rte_strscpy.
In addition, should I replace the call to rte_strscpy with strlcpy?
> -----Original Message-----
> From: Stephen Hemminger <step...@networkplumber.org>
> Sent: Tuesday, 11 February 2025 20:05
> To: Bruce Richardson <bruce.richard...@intel.com>
> Cc: Shani Peretz <shper...@nvidia.com>; dev@dpdk.org; Parav Pandit
> <pa...@nvidia.com>; Xueming Li <xuemi...@nvidia.com>; Nipun Gupta
> <nipun.gu...@amd.com>; Nikhil Agarwal <nikhil.agar...@amd.com>; Hemant
> Agrawal <hemant.agra...@nxp.com>; Sachin Saxena
> <sachin.sax...@nxp.com>; Rosen Xu <rosen...@intel.com>; Chenbo Xia
> <chen...@nvidia.com>; Tomasz Duszynski <tduszyn...@marvell.com>;
> Chengwen Feng <fengcheng...@huawei.com>; NBU-Contact-longli
> (EXTERNAL) <lon...@microsoft.com>; Wei Hu <w...@microsoft.com>; Kevin
> Laatz <kevin.la...@intel.com>; Tyler Retzlaff <roret...@linux.microsoft.com>;
> Jan Blunck <jblu...@infradead.org>
> Subject: Re: [PATCH v5 2/4] lib: fix comparison between devices
>
> External email: Use caution opening links or attachments
>
>
> On Tue, 11 Feb 2025 17:54:26 +0000
> Bruce Richardson <bruce.richard...@intel.com> wrote:
>
> > On Tue, Feb 11, 2025 at 09:48:32AM -0800, Stephen Hemminger wrote:
> > > On Thu, 6 Feb 2025 02:08:36 +0200
> > > Shani Peretz <shper...@nvidia.com> wrote:
> > >
> > > > static int
> > > > -cdx_parse(const char *name, void *addr)
> > > > +cdx_parse(const char *name, void *addr, int *size)
> > > > {
> > > > - const char **out = addr;
> > > > int ret;
> > > >
> > > > ret = strncmp(name, CDX_DEV_PREFIX, strlen(CDX_DEV_PREFIX));
> > > >
> > > > - if (ret == 0 && addr)
> > > > - *out = name;
> > > > + if (ret != 0)
> > > > + return ret;
> > > > +
> > > > + if (size != NULL)
> > > > + *size = strlen(name) + 1;
> > > > +
> > > > + if (addr != NULL)
> > > > + rte_strscpy(addr, name, strlen(name) + 1);
> > >
> > > Why use rte_strscpy() here?
> > >
> > > The intention of strscpy() is to handle case where the resulting
> > > buffer is limited in size. By using the input string length you
> > > aren't really doing anything different than strcpy(). Still unsafe if
> > > output
> (addr) is not big enough.
> >
> > And using strlcpy is probably fine too, without having to use
> > dpdk-specific string functions.
> >
> > /Bruce
>
> The issue is that any length argument needs to come from caller based on the
> size of the target buffer. Not from length of source string.
>
> If you want to make parse code string safe, then either size needs to be
> always
> present and in/out parameter or need to have a src_size and resulting size as
> separate params.
