[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    Re: [PATCH 24.03 v2 1/5] arg_parser: new library for command line parsing
From:       Bruce Richardson <bruce.richardson () intel ! com>
Date:       2023-11-30 8:59:33
Message-ID: ZWhO9Tgm77UoeuXf () bricha3-MOBL ! ger ! corp ! intel ! com
[Download RAW message or body]

On Wed, Nov 29, 2023 at 02:14:13PM -0800, Stephen Hemminger wrote:
> On Tue, 28 Nov 2023 14:07:41 +0000
> Euan Bourke <euan.bourke@intel.com> wrote:
> 
> > +int
> > +rte_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len)
> > +{
> > +	int32_t min = -1;
> > +	char *end = NULL;
> > +
> > +	struct core_bits *mask = malloc(sizeof(struct core_bits));
> > +	if (mask == NULL)
> > +		return -1;
> > +	memset(mask, 0, sizeof(struct core_bits));
> > +
> 
> The structure is not variable size, and small, why bother using malloc().
> Just use on stack structure.

Well, it's just over 4k in size, and we hit problems in the past with
alpine linux where the stack size wasn't as big as expected, leading to
issues with telemetry string manipulation. In that case I think it was due
to nested calls as well as large stack structs, though. For this core
parsing, having this struct on stack is unlikely to cause issues, but for a
non-datapath API I suggests to Euan to use malloc as a lower-risk
alternative, since performance of an arg parsing API is unlikely to be a
major concern.

However, if you feel that it should be on the stack and won't cause any
issues, it does make the code shorter and easier to work with!

/Bruce
[prev in list] [next in list] [prev in thread] [next in thread]