<div dir="ltr"><div><div><div><div>> long error messages that we cannot split into a multiline thing anyway.<br><br></div>I split numerous Orangefs ones at substitution characters, which<br></div>didn't break grepability. No one complained, and sure made<br></div>stuff look nicer to me. You might be able to do that some places?<br><br></div>-Mike<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 6, 2016 at 12:50 PM, Oleg Drokin <span dir="ltr"><<a href="mailto:oleg.drokin@intel.com" target="_blank">oleg.drokin@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
On Dec 6, 2016, at 11:15 AM, Greg KH wrote:<br>
<br>
> On Tue, Dec 06, 2016 at 10:34:03AM -0500, Oleg Drokin wrote:<br>
>><br>
>> On Dec 3, 2016, at 3:55 AM, <a href="mailto:gregkh@linuxfoundation.org">gregkh@linuxfoundation.org</a> wrote:<br>
>><br>
>>> On Fri, Dec 02, 2016 at 09:53:08PM +0000, Dilger, Andreas wrote:<br>
>>>> Al,<br>
>>>> Greg recently raised the issue of what still needs to be done to<br>
>>>> move the Lustre code out of staging/ and into the fs/ tree.<br>
>>>><br>
>>>> James has been doing a great job of cleaning up various checkpatch<br>
>>>> issues and keeping the code updated with the latest fixes, but we<br>
>>>> were wondering what you were aware of that needed to be cleaned<br>
>>>> up in Lustre?<br>
>>><br>
>>> Is the whole "mixing kernel structures in userspace structures" all<br>
>>> resolved now?  For some reason I thought that you had kernel locks being<br>
>>> passed to userspace and then back into the kernel, but it's been a long<br>
>>> time since I last looked...<br>
>><br>
>> While we certainly had our share of mixing user/kernelspace structures,<br>
>> I don't think we ever passed anything with locks around back and forth.<br>
>><br>
>> I just did a brief check and I don't see anything glaring on this particular front.<br>
>><br>
>>> If you feel you are ready for a "real" review, I'll be glad to go over<br>
>>> the code before the vfs people look at it, just let me know.  No need to<br>
>>> bother them if you still have basic things wrong that I can find…<br>
>><br>
>> I think this would be beneficial at this stage.<br>
><br>
> I see loads of <a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a> warnings and a few errors, how about fixing<br>
> all of them up first?<br>
><br>
<br>
</span>There are 16 errors,<br>
of them:<br>
8 are false positives in  drivers/staging/lustre/lnet/<wbr>libcfs/hash.c<br>
1 false positive in drivers/staging/lustre/lnet/<wbr>libcfs/tracefile.h<br>
1 false positive in drivers/staging/lustre/lustre/<wbr>include/lustre/lustre_idl.h<br>
1 I think false positive in drivers/staging/lustre/lustre/<wbr>include/lustre/lustre_idl.h (PLDLMRES)<br>
2 false positives in drivers/staging/lustre/lustre/<wbr>include/lustre/lustre_user.h<br>
<br>
an error I don't really understand, what does it want us to do here?<br>
ERROR: trailing statements should be on next line<br>
#196951: FILE: drivers/staging/lustre/lustre/<wbr>mdc/mdc_request.c:1063:<br>
+                       for (end_dirent = ent; ent;<br>
+                            end_dirent = ent, ent = lu_dirent_next(ent));<br>
<br>
leaving 2 real ones, one missing space in drivers/staging/lustre/lnet/<wbr>klnds/o2iblnd/o2iblnd_cb.c<br>
and 1 spaces instead of tabs in drivers/staging/lustre/lnet/<wbr>klnds/socklnd/socklnd.h<br>
<br>
I'll have patches shortly for these.<br>
<br>
On the warning front, we have 879 line over 80 characters, but a bunch<br>
of those are due to long error messages that we cannot split into a multiline thing anyway.<br>
71 Prefer 'unsigned int' to bare use of 'unsigned' that is a relatively new one<br>
22 new typedefs (all in lnet, I drove out all the lustre ones, so I guess I can tackle these too).<br>
a bunch of comment style problems<br>
a few "function definition argument … should also have an identifier name" that<br>
seems to be a new one too, I don't remember seeing this before.<br>
<br>
And a bunch of rarer ones for the total of 1243 (823 in just Lustre).<br>
Multiple classes of them are actually not easily fixable or false positives too.<br>
Do you really want all of these fixed too somehow?<br>
<div class="HOEnZb"><div class="h5"><br>
--<br>
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in<br>
the body of a message to <a href="mailto:majordomo@vger.kernel.org">majordomo@vger.kernel.org</a><br>
More majordomo info at  <a href="http://vger.kernel.org/majordomo-info.html" rel="noreferrer" target="_blank">http://vger.kernel.org/<wbr>majordomo-info.html</a><br>
</div></div></blockquote></div><br></div>