| Message ID | 1472532619-22170-2-git-send-email-nayna@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 30, 2016 at 12:50:13AM -0400, Nayna Jain wrote: > Open methods for eventlog ascii and binary bios measurements file > operations are very similar. This patch refactors the code into > single open() call by passing seq_operations as i_node->private data. > > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > --- > drivers/char/tpm/tpm_eventlog.c | 59 +++++++++-------------------------------- > 1 file changed, 13 insertions(+), 46 deletions(-) > > diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c > index e722886..b0a4d02 100644 > --- a/drivers/char/tpm/tpm_eventlog.c > +++ b/drivers/char/tpm/tpm_eventlog.c > @@ -7,6 +7,7 @@ > * Stefan Berger <stefanb@us.ibm.com> > * Reiner Sailer <sailer@watson.ibm.com> > * Kylene Hall <kjhall@us.ibm.com> > + * Nayna Jain <nayna@linux.vnet.ibm.com> > * > * Maintained by: <tpmdd-devel@lists.sourceforge.net> > * > @@ -318,12 +319,14 @@ static const struct seq_operations tpm_binary_b_measurments_seqops = { > .show = tpm_binary_bios_measurements_show, > }; > > -static int tpm_ascii_bios_measurements_open(struct inode *inode, > +static int tpm_bios_measurements_open(struct inode *inode, > struct file *file) > { > int err; > struct tpm_bios_log *log; > struct seq_file *seq; > + const struct seq_operations *seqops = > + (const struct seq_operations *)inode->i_private; Indentation missing. > > log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL); > if (!log) > @@ -333,7 +336,7 @@ static int tpm_ascii_bios_measurements_open(struct inode *inode, > goto out_free; > > /* now register seq file */ > - err = seq_open(file, &tpm_ascii_b_measurments_seqops); > + err = seq_open(file, seqops); > if (!err) { > seq = file->private_data; > seq->private = log; > @@ -349,46 +352,8 @@ out_free: > goto out; > } > > -static const struct file_operations tpm_ascii_bios_measurements_ops = { > - .open = tpm_ascii_bios_measurements_open, > - .read = seq_read, > - .llseek = seq_lseek, > - .release = tpm_bios_measurements_release, > -}; > - > -static int tpm_binary_bios_measurements_open(struct inode *inode, > - struct file *file) > -{ > - int err; > - struct tpm_bios_log *log; > - struct seq_file *seq; > - > - log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL); > - if (!log) > - return -ENOMEM; > - > - if ((err = read_log(log))) > - goto out_free; > - > - /* now register seq file */ > - err = seq_open(file, &tpm_binary_b_measurments_seqops); > - if (!err) { > - seq = file->private_data; > - seq->private = log; > - } else { > - goto out_free; > - } > - > -out: > - return err; > -out_free: > - kfree(log->bios_event_log); > - kfree(log); > - goto out; > -} > - > -static const struct file_operations tpm_binary_bios_measurements_ops = { > - .open = tpm_binary_bios_measurements_open, > +static const struct file_operations tpm_bios_measurements_ops = { > + .open = tpm_bios_measurements_open, > .read = seq_read, > .llseek = seq_lseek, > .release = tpm_bios_measurements_release, > @@ -413,15 +378,17 @@ struct dentry **tpm_bios_log_setup(const char *name) > > bin_file = > securityfs_create_file("binary_bios_measurements", > - S_IRUSR | S_IRGRP, tpm_dir, NULL, > - &tpm_binary_bios_measurements_ops); > + S_IRUSR | S_IRGRP, tpm_dir, > + (void *)&tpm_binary_b_measurments_seqops, > + &tpm_bios_measurements_ops); > if (is_bad(bin_file)) > goto out_tpm; > > ascii_file = > securityfs_create_file("ascii_bios_measurements", > - S_IRUSR | S_IRGRP, tpm_dir, NULL, > - &tpm_ascii_bios_measurements_ops); > + S_IRUSR | S_IRGRP, tpm_dir, > + (void *)&tpm_ascii_b_measurments_seqops, > + &tpm_bios_measurements_ops); > if (is_bad(ascii_file)) > goto out_bin; Otherwise fine. Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > -- > 2.5.0 > > > ------------------------------------------------------------------------------ > _______________________________________________ > tpmdd-devel mailing list > tpmdd-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tpmdd-devel /Jarkko ------------------------------------------------------------------------------
On Tue, Aug 30, 2016 at 12:50:13AM -0400, Nayna Jain wrote: > Open methods for eventlog ascii and binary bios measurements file > operations are very similar. This patch refactors the code into > single open() call by passing seq_operations as i_node->private data. > > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> Looks basically fine, with Jarkko's comment addressed. I recommend using clang-format when working with the kernel, it makes everything easy. Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Jason ------------------------------------------------------------------------------
Thanks Jason for review. I will address your comments in my next version of patches. I also have some thoughts on one of your comment.. Responding inline in respective patch. Thanks & Regards, - Nayna On 08/30/2016 10:33 PM, Jason Gunthorpe wrote: > On Tue, Aug 30, 2016 at 12:50:13AM -0400, Nayna Jain wrote: >> Open methods for eventlog ascii and binary bios measurements file >> operations are very similar. This patch refactors the code into >> single open() call by passing seq_operations as i_node->private data. >> >> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> > > Looks basically fine, with Jarkko's comment addressed. I recommend > using clang-format when working with the kernel, it makes everything > easy. > > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Jason > ------------------------------------------------------------------------------
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c index e722886..b0a4d02 100644 --- a/drivers/char/tpm/tpm_eventlog.c +++ b/drivers/char/tpm/tpm_eventlog.c @@ -7,6 +7,7 @@ * Stefan Berger <stefanb@us.ibm.com> * Reiner Sailer <sailer@watson.ibm.com> * Kylene Hall <kjhall@us.ibm.com> + * Nayna Jain <nayna@linux.vnet.ibm.com> * * Maintained by: <tpmdd-devel@lists.sourceforge.net> * @@ -318,12 +319,14 @@ static const struct seq_operations tpm_binary_b_measurments_seqops = { .show = tpm_binary_bios_measurements_show, }; -static int tpm_ascii_bios_measurements_open(struct inode *inode, +static int tpm_bios_measurements_open(struct inode *inode, struct file *file) { int err; struct tpm_bios_log *log; struct seq_file *seq; + const struct seq_operations *seqops = + (const struct seq_operations *)inode->i_private; log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL); if (!log) @@ -333,7 +336,7 @@ static int tpm_ascii_bios_measurements_open(struct inode *inode, goto out_free; /* now register seq file */ - err = seq_open(file, &tpm_ascii_b_measurments_seqops); + err = seq_open(file, seqops); if (!err) { seq = file->private_data; seq->private = log; @@ -349,46 +352,8 @@ out_free: goto out; } -static const struct file_operations tpm_ascii_bios_measurements_ops = { - .open = tpm_ascii_bios_measurements_open, - .read = seq_read, - .llseek = seq_lseek, - .release = tpm_bios_measurements_release, -}; - -static int tpm_binary_bios_measurements_open(struct inode *inode, - struct file *file) -{ - int err; - struct tpm_bios_log *log; - struct seq_file *seq; - - log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL); - if (!log) - return -ENOMEM; - - if ((err = read_log(log))) - goto out_free; - - /* now register seq file */ - err = seq_open(file, &tpm_binary_b_measurments_seqops); - if (!err) { - seq = file->private_data; - seq->private = log; - } else { - goto out_free; - } - -out: - return err; -out_free: - kfree(log->bios_event_log); - kfree(log); - goto out; -} - -static const struct file_operations tpm_binary_bios_measurements_ops = { - .open = tpm_binary_bios_measurements_open, +static const struct file_operations tpm_bios_measurements_ops = { + .open = tpm_bios_measurements_open, .read = seq_read, .llseek = seq_lseek, .release = tpm_bios_measurements_release, @@ -413,15 +378,17 @@ struct dentry **tpm_bios_log_setup(const char *name) bin_file = securityfs_create_file("binary_bios_measurements", - S_IRUSR | S_IRGRP, tpm_dir, NULL, - &tpm_binary_bios_measurements_ops); + S_IRUSR | S_IRGRP, tpm_dir, + (void *)&tpm_binary_b_measurments_seqops, + &tpm_bios_measurements_ops); if (is_bad(bin_file)) goto out_tpm; ascii_file = securityfs_create_file("ascii_bios_measurements", - S_IRUSR | S_IRGRP, tpm_dir, NULL, - &tpm_ascii_bios_measurements_ops); + S_IRUSR | S_IRGRP, tpm_dir, + (void *)&tpm_ascii_b_measurments_seqops, + &tpm_bios_measurements_ops); if (is_bad(ascii_file)) goto out_bin;
Open methods for eventlog ascii and binary bios measurements file operations are very similar. This patch refactors the code into single open() call by passing seq_operations as i_node->private data. Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com> --- drivers/char/tpm/tpm_eventlog.c | 59 +++++++++-------------------------------- 1 file changed, 13 insertions(+), 46 deletions(-)