Hi Jason,
Thanks for the review.
On 07/29/2016 10:27 PM, Jason Gunthorpe wrote:
> On Fri, Jul 29, 2016 at 02:44:38AM -0400, Nayna Jain wrote:
>> Refactored eventlog.c file into tpm_eventlog.c and tpm_eventlog_init.c
>
> If you are going to work on this stuff (and have the ability to test
> it) can you fix some of the generic pre-existing problems too?
Can you please help me to understand what in particular are you
referring to by "some of the generic pre-existing problems" ?
>
>> +static int tpm_ascii_bios_measurements_open(struct inode *inode,
>> + struct file *file)
>
>> +static int tpm_binary_bios_measurements_open(struct inode *inode,
>> + struct file *file)
>
> We don't need two (or more!) identical versions of this function, and it
> is easy to fix:
Sure, will fix it.
>
>> + bin_file =
>> + securityfs_create_file("binary_bios_measurements",
>> + S_IRUSR | S_IRGRP, tpm_dir, NULL,
>> + &tpm_binary_bios_measurements_ops);
>
> Replace NULL with &tpm_binary_b_measurments_seqops and recover it in
> the generic open using the inode->i_private pointer.
Sure, will fix it.
>
>> + ret = kmalloc(3 * sizeof(struct dentry *), GFP_KERNEL);
>> + if (!ret)
>> + goto out_ascii;
>
> I can't find a kfree for this memory, looks like it is leaking, please
> fix it.
>
> Do not allocate memory for this, just include the dentry array
> directly in the tpm_chip as the sysfs does today.
Do you mean here to define it as struct dentry *bios_dir[3] as member of
struct tpm_chip ?
>
> You can change the signatures to accept tpm_chip in a cleanup patch as
> well, moving from the tpm2 patch.
>
Ok. Will address these comments in the next version of the patch I will
be posting.
Thanks & Regards,
- Nayna
> Jason
>
------------------------------------------------------------------------------
_______________________________________________
tpmdd-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
