On 11/27/13 at 11:20am, Matt Fleming wrote: > On Tue, 26 Nov, at 01:57:55PM, Dave Young wrote: > > +Users: > > + Kexec Mailing List <kexec at lists.infradead.org> > > "Kexec" please. Will change > > > +static ssize_t version_show(struct kobject *kobj, > > + struct kobj_attribute *attr, char *buf) > > This is pretty strange indentation. Usually we prefer to line up the > arguments like you've done elsewhere in this file. Ok. > > > +{ > > + return sprintf(buf, "0x%04x\n", boot_params.hdr.version); > > +} > > + > > +static struct kobj_attribute boot_params_version_attr = __ATTR_RO(version); > > + > > +static ssize_t boot_params_data_read(struct file *fp, struct kobject *kobj, > > + struct bin_attribute *bin_attr, > > + char *buf, loff_t off, size_t count) > > +{ > > + memcpy(buf, (void *)&boot_params + off, count); > > + return count; > > +} > > Don't we need some checks here to ensure we don't read past the end of > boot_params? The size was added below, so I would assume file read will not beyond the file size: .size = sizeof(boot_params), > > > +static ssize_t type_show(struct kobject *kobj, > > + struct kobj_attribute *attr, char *buf) > > More whacky indentation. Will fix > > > +static int __init create_setup_data_node(struct kobject *parent, > > + struct kobject **kobjp, int nr) > > Funky indentation. Will fix Thanks for review Dave