Re: [PATCH RESEND 2/3] tpm: replace dynamically allocated bios_dir with a static array

From: Nayna
Date: Tue Oct 11 2016 - 13:21:11 EST




On 10/11/2016 10:23 PM, Jason Gunthorpe wrote:
On Tue, Oct 11, 2016 at 02:23:15PM +0300, Jarkko Sakkinen wrote:
+ chip->bios_dir[cnt] =
securityfs_create_file("ascii_bios_measurements",
- S_IRUSR | S_IRGRP, tpm_dir,
+ S_IRUSR | S_IRGRP, chip->bios_dir[0],
(void *)&tpm_ascii_b_measurments_seqops,
&tpm_bios_measurements_ops);

+ if (is_bad(chip->bios_dir[cnt]))
+ goto err;

+err:
+ chip->bios_dir[cnt] = NULL;

The updated patch looks fine.
Just, I am not sure if NULL assignment is needed.

It's not needed.

It is required to switch an ERR_PTR to NULL, see is_bad()

My understanding is that securityfs_remove() takes care of both NULL and ERR_PTR().

From securityfs_remove():

if (!dentry || IS_ERR(dentry))
return;

Thanks & Regards,
- Nayna


The original version that used a counter in chip did not need it.

Jason