Re: [dpdk-dev] [PATCH] eal: fix strdup usages in internal config

[Thomas Monjalon <thomas@monjalon.net>]

31/01/2019 15:15, Kevin Traynor:
> On 01/31/2019 02:10 PM, Burakov, Anatoly wrote:
> > On 31-Jan-19 11:21 AM, Kevin Traynor wrote:
> >> On 01/10/2019 01:38 PM, Anatoly Burakov wrote:
> >>> Currently, we use strdup in a few places to store command-line
> >>> parameter values for certain internal config values. There are
> >>> several issues with that.
> >>>
> >>> First of all, they're never freed, so memory ends up leaking
> >>> either after EAL exit, or when these command-line options are
> >>> supplied multiple times.
> >>>
> >>> Second of all, they're defined as `const char *`, so they
> >>> *cannot* be freed even if we wanted to.
> >>>
> >>> Finally, strdup may return NULL, which will be stored in the
> >>> config. For most fields, NULL is a valid value, but for the
> >>> default prefix, the value is always expected to be valid.
> >>>
> >>> To fix all of this, three things are done. First, we change
> >>> the definitions of these values to `char *` as opposed to
> >>> `const char *`. This does not break the ABI, and previous
> >>> code assumes constness (which is more restrictive), so it's
> >>> safe to do so.
> >>>
> >>> Then, fix all usages of strdup to check return value, and add
> >>> a cleanup function that will free the memory occupied by
> >>> these strings, as well as freeing them before assigning a new
> >>> value to prevent leaks when parameter is specified multiple
> >>> times.
> >>>
> >>> And finally, add an internal API to query hugefile prefix, so
> >>> that, absent of a valid value, a default value will be
> >>> returned, and also fix up all usages of hugefile prefix to
> >>> use this API instead of accessing hugefile prefix directly.
> >>>
> >>> Bugzilla ID: 108
> >>>
> >>
> >> Hi Anatoly - this doesn't have stable or Fixes tags, but the bugzilla
> >> was reported on 17.11. Is it for backport to stable branches?
> >>
> > 
> > It can be. Whether it's worth the effort of backporting is not my call :)
> > 
> 
> It's fine for 18.11 branch anyway, just needed a little help due to some
> changed context. I will send diff to stable list as normal.

Nothing was broken. I see it like an improvement.
Not sure it is worth the effort.