On 20/03/24 13:07, Mimi Zohar wrote: > >>>> diff --git a/security/integrity/ima/ima_fs.c >>>> b/security/integrity/ima/ima_fs.c >>>> index cd1683dad3bf..475ab368e32f 100644 >>>> --- a/security/integrity/ima/ima_fs.c >>>> +++ b/security/integrity/ima/ima_fs.c >>>> @@ -116,9 +116,13 @@ void ima_putc(struct seq_file *m, void *data, int >>>> datalen) >>>> seq_putc(m, *(char *)data++); >>>> } >>>> >>>> +static struct dentry **ima_ascii_measurements_files; >>>> +static struct dentry **ima_binary_measurements_files; >>> >>> The variable naming isn't quite right. It's defined as a 'struct dentry', >>> but >>> the name is '*_files'. Why not just name the variables 'ima_{ascii, binary} >>> _measurements'? >> >> Hi Mimi, > > Hi Enrico, > >> thank you for pointing that out. What do you think of naming them 'ima_{ascii, >> binary}_securityfs_measurement_lists', to have also coherence with the names >> of >> the new functions defined. > > As these are static variables, prefixing them with 'ima_' isn't necessary. > Either way is fine. Hi Mimi, perfect, in this way they would be even shorter. Thank you, Enrico >>>> +static void remove_measurements_list_files(struct dentry **files) >>> >>> And remove '_files' from the function name. Perhaps rename it >>> remove_measurement_lists or remove_securityfs_measurement_lists. >>> >>>> +{ >>>> + int i; >>>> + >>>> + if (files) { >>>> + for (i = 0; i < ima_measurements_files_count; i++) >>>> + securityfs_remove(files[i]); >>>> + >>>> + kfree(files); >>>> + } >>>> +} >>>> + >>>> +static int create_measurements_list_files(void) >>> >>> And remove '_files' from the function name. Perhaps rename it to >>> create_measurement_lists or create_securityfs_measurement_lists. >> >> I think that keeping this structure for the names >> 'remove_securityfs_measurement_lists' and >> 'create_securityfs_measurement_lists' >> makes sense. > > Agreed. > > thanks, > > Mimi