Re: [PATCH v5 03/13] xen/arm: add Dom0 cache coloring support

[Julien Grall <julien@xxxxxxx>]

Hi Carlo,

On 08/01/2024 10:06, Carlo Nonato wrote:
> > One of the reason is at least in the dom0less case, you will override
> > the value afterwards.
> In that case I need to allocate the array before parsing the string.
> I allocate a full array then the string is parsed and the actual size is found
> at the end of this phase. Knowing the actual size would require two parsing
> stages. Yes I'm wasting a bit of memory by oversizing the array here. Is it
> a problem?
While wasting memory is indeed not nice. This wasn't the main reason of 
this comment.
The reason is that you seem to set d->num_lcc_colors will but will never 
be read before it gets overwritten. Looking again at the code, you are 
also assuming parse_colors() will always take an array of nr_colors.
It would be better if parse_colors() takes the maximum size of the array 
in parameter. This would harden the code and it makes more sense for 
domain_alloc_colors() to set d->num_lcc_colors.
Also, I just noticed you have a global variable named nr_colors and the 
function parse_colors() takes an argument called *num_colors. This is 
quite confusing, can we have better name?
Maybe rename nr_colors to nr_global_colors and and num_colors to 
nr_array_colors?
Cheers,

--
Julien Grall