On Fri, Oct 25, 2013 at 2:59 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> This is for future reference, the current patch seems fine.
>
> On Fri, Oct 25, 2013 at 01:20:56PM -0700, Lisa Nguyen wrote:
>> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
>> @@ -159,7 +159,7 @@ static int lprocfs_ns_resources_seq_show(struct seq_file *m, void *v)
>> {
>> struct ldlm_namespace *ns = m->private;
>> __u64 res = 0;
>> - cfs_hash_bd_t bd;
>> + struct cfs_hash_bd bd;
>> int i;
>
> These are badly aligned. I assume you are using sed. Also there is an
> extra space before the '=' on the first line and "i" is out of
> alignment. So it was pretty ugly to begin with.
True. I used sed and didn't think about fixing the spacing for the
other variables, left them the way they were. Will keep this in mind
for next time.
> Really, don't use tabs, just use a single space.
>
> struct ldlm_namespace *ns = m->private;
> u64 res = 0;
> struct cfs_hash_bd bd;
> int i;
>
> If you do it like that then later when you need to update the types then
> you don't need to re-indent everything to match.
>
> The difference between u64 and __u64 is that you have to use __u64 in
> headers which are exported to userspace. This isn't a header so that
> doesn't apply.
Ah, another thing I just learned :)
I always appreciate your feedback and information that you want to
share with me. It helps me to continue submitting quality patches.
Thanks,
Lisa
> I haven't totally understood which lustre files are expected to be uapi.
> Those should probably be more minimal and maybe in a separate directory.
>
> regards,
> dan carpenter
>
>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
