Received-SPF: pass (google.com: domain of linux-kernel+bounces-111880-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <8964b621-657d-4f9c-aeb0-3d3ed8c62c3f@intel.com>
Date: Fri, 22 Mar 2024 10:44:32 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 11/13] selftests/resctrl: Convert ctrlgrp & mongrp to
 pointers
Content-Language: en-US
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
CC: <linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>, "Babu
 Moger" <babu.moger@amd.com>, =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?=
	<maciej.wieczor-retman@intel.com>, Fenghua Yu <fenghua.yu@intel.com>, LKML
	<linux-kernel@vger.kernel.org>
References: <20240311135230.7007-1-ilpo.jarvinen@linux.intel.com>
 <20240311135230.7007-12-ilpo.jarvinen@linux.intel.com>
 <578d0b55-c51a-49d1-8f54-989215a3a4b8@intel.com>
 <93e4f096-47df-9eba-095f-e8a8c3cd04f5@linux.intel.com>
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <93e4f096-47df-9eba-095f-e8a8c3cd04f5@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UFhpbkx4aXFlWDdERzdoOHJXOStBRmR4eUNRd0J6dzNQUUpCUzYzTzROSnhj?=
 =?utf-8?B?Z2ZTUExXclZuczRrWi9RanM4ZkdyS3ovKzNTejJXQUc0bG54ME40NnU1N0ZE?=
 =?utf-8?B?enJLSGN2WUNvb0xjZWJkdEdhSkdOeEFBbWw5Q3ZlUDB0L0wrZW1kc3VLOXQv?=
 =?utf-8?B?c2IwVnpXc21GRmUyK09CQmY4T1pjQmhVS1pOQThLU1ZqN2xpUU9BeW1OR2xW?=
 =?utf-8?B?UXFuUUdxa1d3MnJiMXoya2w1Q1JwbVlRY2ZjRE5XZjVqNFpGdlhTSnZOUnhJ?=
 =?utf-8?B?cGd1cDkxVU1HYnVyTW1KeTRFUlcrVnBLYlV3aFRJMW1XWUM0WS9sMFBuZkRB?=
 =?utf-8?B?TmdKUUlUTjZxZ2xVV1ZGUWtPbm9LQzZBVUFKMmVQb1ErSFovTERXUURsNTZB?=
 =?utf-8?B?K0hZSWo1aUxadG9jWldJSVVSRFJLZHNuL0ZWVWYybnRkOUlNdlJIMEF5WTJr?=
 =?utf-8?B?WEV2bFhNOHBnRHNuTVdXeFQ5bUhjTmhuK08xaFhVUzc2UUpwK2RRVFQyMjM2?=
 =?utf-8?B?VW4yZXBORVRhTWgvRGlIazNOK21Rc0krZzFhM0tpTm95aS9ieENSODlWV3Ez?=
 =?utf-8?B?bGNGQTdrdVJJcldiZlMxdEx1VGM5ekZQK09GRlg0KzRZTWhxeE05TFppWlVk?=
 =?utf-8?B?WFQrZ3p4UzFvc2J2VjRYS0FKOEpNc2IrWXBaNDlCZzFEMTNMQUswSzMwMEJF?=
 =?utf-8?B?S1YvcHMwZlpac1dWek9hM2NEMW9QeE5TcFdoSmREL1VBR1dFc25VQkF3dXNx?=
 =?utf-8?B?a3dSMm1HNW1rZnJUUzFESzhWRm1DbFQ0cC9UV1BVa0pUVks0eGRUNUlKejkr?=
 =?utf-8?B?RUQxejNJZko1SVNMTWNsTE1NVXlpbkxVL0swQmJVMit4RHpkN1pydk9xRms4?=
 =?utf-8?B?WW54K2kzMWh6c3FoM0FBU1FGR0NkTkZjd2JPeGNvWHhRb1lkSzBxWVZ0Mlht?=
 =?utf-8?B?Z0ZSUkh1dThQU0dBa0xoRUh1U0pzRUEwaVU5VG8yaHdKNXBTdVMxR0ZNSmt4?=
 =?utf-8?B?NlcrUlFKU3NQRjIvaDJKVklEN0JsbUZONVJ0S21ZVG9haE1CSUROMDRHL3U0?=
 =?utf-8?B?MTRQV3lLUDYvdStzWllOWDZlbDIzTHI2NEhTMTlDR3hpc3d1U2FOdnhvMmpI?=
 =?utf-8?B?QnZkaWFvOXRzTlNtMjlTUWFLb3l1VXhKUENVbS8rekgzWXFBVW9tRHNiOXJz?=
 =?utf-8?B?dGhXUWsxNnZteHRMQnQraTVWQ1VDVTliR0VZcHdvaGM2WjZuYWdKWm9oOXkz?=
 =?utf-8?B?eHg4Tml3blVaL0haMXMwbno1UDBTRitzbk5sTURvbFB0MTRyVS9oektlVE1h?=
 =?utf-8?B?V2lONkg2L01zcGJBRFhzZDRWWDNOdmg2dzdEdDZMOVkxektGL0FQUG9sWk94?=
 =?utf-8?B?UjkrQTZBVnJscGtYcUhBK1hQMEZDNFc3ZW51b3d2dzl5bXBNeUFBSWQ4T1lQ?=
 =?utf-8?B?cWdzNEQ2N3R6aHhueWxBMDB1cGNSazFRZnRqUFdsdFlhL3Q5TW9uaCtUVThz?=
 =?utf-8?B?OWx0MkNDaUM3NDBIMkp2dDYyMWl5QU9ybW9ZSUpzdTJ4Z1NhRWRtMWpHcTA0?=
 =?utf-8?B?KzdLSC9xd2prNDFwYTF2SEhKUExNelllSkUxandZTTBjS0I5Q2lVekxnbSto?=
 =?utf-8?B?N3pBYWRRVkJnaGtpMi8zRG9KOFBFWVhFNG4zZFBaT0puOG02Tnd1Z0FWUVhS?=
 =?utf-8?B?b0ppMDN5T2ZxQ2xBTXdtZnJkczlPUXVKeFJVakl0VzdjWm5Vd1FvYnBEVGZN?=
 =?utf-8?B?di9GRkxWeXI1UzV5dFZsekxSRnRxN1QvUzhRWGV1Z3ppQ1NvRkw3cmJJeFJM?=
 =?utf-8?B?OFhaNDd0Sng1WTFIY2NYeEpGcjhhdVI4cnY1UmpUdVVPQ3BRR2RRSllqazk4?=
 =?utf-8?B?cjE5dFhqelZuN0QyTElTZlIvRm85NU56RWk2M0RSZzVLa1ZuMXBoNERRVFkw?=
 =?utf-8?B?dWNxVU83VFZFZnFwbGt3cWs5d0VjbCtVcFdCV3AvSExBS0g2U0NmdTRYVWcx?=
 =?utf-8?B?RU9HcWFWUlVMMWM2b0l3RDRoWW1iZVVWYUtEdUJIYjdpUlBmcHRVamY1UlIx?=
 =?utf-8?B?QjZjTS80V1VwZFlvQUs4QlJCU3ZEU1VoUDltdUYrNlpFRGdWWjNRUlR5VVNt?=
 =?utf-8?B?MUwrVWxVUmJyRzZSNG1yNC8xa3NpZ0xZeWRUZDRibXp6SVBmUGs2OCtELy8z?=
 =?utf-8?B?dEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 083aa896-37a4-460b-0922-08dc4a97bb9f
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Mar 2024 17:44:32.9562
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: G1siO+QfhrxRZptOaI4Uqzz9lzSeeuXKC9jlE89KPOTTOeJmVQwABEqQARuIl1BfGVxR442F/q661E6W3rcG/4Y29UdgF9gE09MIBv2hpao=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6950
X-OriginatorOrg: intel.com

Hi Ilpo,

On 3/22/2024 5:30 AM, Ilpo Järvinen wrote:
> On Wed, 20 Mar 2024, Reinette Chatre wrote:
>> On 3/11/2024 6:52 AM, Ilpo Järvinen wrote:
>>> The struct resctrl_val_param has control and monitor groups as char
>>> arrays but they are not supposed to be mutated within resctrl_val().
>>>
>>> Convert the ctrlgrp and mongrp char array within resctrl_val_param to
>>> plain const char pointers and adjust the strlen() based checks to
>>> check NULL instead.
>>>
>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>> ---
>>>  tools/testing/selftests/resctrl/resctrl.h   | 4 ++--
>>>  tools/testing/selftests/resctrl/resctrlfs.c | 8 ++++----
>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
>>> index 52769b075233..54e5bce4c698 100644
>>> --- a/tools/testing/selftests/resctrl/resctrl.h
>>> +++ b/tools/testing/selftests/resctrl/resctrl.h
>>> @@ -89,8 +89,8 @@ struct resctrl_test {
>>>   */
>>>  struct resctrl_val_param {
>>>  	char		*resctrl_val;
>>> -	char		ctrlgrp[64];
>>> -	char		mongrp[64];
>>> +	const char	*ctrlgrp;
>>> +	const char	*mongrp;
>>>  	char		filename[64];
>>>  	unsigned long	mask;
>>>  	int		num_of_runs;
>>> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
>>> index 79cf1c593106..dbe0cc6d74fa 100644
>>> --- a/tools/testing/selftests/resctrl/resctrlfs.c
>>> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
>>> @@ -469,7 +469,7 @@ static int create_grp(const char *grp_name, char *grp, const char *parent_grp)
>>>  	 * length of grp_name == 0, it means, user wants to use root con_mon
>>>  	 * grp, so do nothing
>>>  	 */
>>
>> Could you please confirm that the comments are still accurate?
> 
> It's not, I missed it.
> 
>>> -	if (strlen(grp_name) == 0)
>>> +	if (!grp_name)
>>>  		return 0;
> 
> But now when looking into the surrounding code, to me it looks the correct 
> action here is to remove the comment and return -1 instead of 0. It makes
> this just an internal sanity check that grp_name is provided by the 
> caller.
> 

hmmm ... this should not be an error because the caller is not required
to provide grp_name. Not providing grp_name has a specific meaning
of this operating on the CON_MON group and a failure would break flows
operating on the CON_MON group.

Reinette