Re: [PATCH 03/34] brcmfmac: firmware: Support having multiple alt paths

[Dmitry Osipenko]

02.01.2022 17:18, Hector Martin пишет:
> On 2022/01/02 15:45, Dmitry Osipenko wrote:
>> 26.12.2021 18:35, Hector Martin пишет:
>>> -static char *brcm_alt_fw_path(const char *path, const char *board_type)
>>> +static const char **brcm_alt_fw_paths(const char *path, const char *board_type)
>>>  {
>>>  	char alt_path[BRCMF_FW_NAME_LEN];
>>> +	char **alt_paths;
>>>  	char suffix[5];
>>>  
>>>  	strscpy(alt_path, path, BRCMF_FW_NAME_LEN);
>>> @@ -609,27 +612,46 @@ static char *brcm_alt_fw_path(const char *path, const char *board_type)
>>>  	strlcat(alt_path, board_type, BRCMF_FW_NAME_LEN);
>>>  	strlcat(alt_path, suffix, BRCMF_FW_NAME_LEN);
>>>  
>>> -	return kstrdup(alt_path, GFP_KERNEL);
>>> +	alt_paths = kzalloc(sizeof(char *) * 2, GFP_KERNEL);
>>
>> array_size()?
> 
> Of what array?

array_size(sizeof(*alt_paths), 2)

>>> +	alt_paths[0] = kstrdup(alt_path, GFP_KERNEL);
>>> +
>>> +	return (const char **)alt_paths;
>>
>> Why this casting is needed?
> 
> Because implicit conversion from char ** to const char ** is not legal
> in C, as that could cause const unsoundness if you do this:
> 
> char *foo[1];
> const char **bar = foo;
> 
> bar[0] = "constant string";
> foo[0][0] = '!'; // clobbers constant string

It's up to a programmer to decide what is right to do. C gives you
flexibility, meanwhile it's easy to shoot yourself in the foot if you
won't be careful.

> But it's fine in this case since the non-const pointer disappears so
> nothing can ever write through it again.
> 

There is indeed no need for the castings in such cases, it's a typical
code pattern in kernel. You would need to do the casting for the other
way around, i.e. if char ** was returned and **alt_paths was a const.