[Top] [All Lists]

Re: [PATCH] xfs: stop using simple_strtoul()

To: Abhijit Pawar <abhi.c.pawar@xxxxxxxxx>
Subject: Re: [PATCH] xfs: stop using simple_strtoul()
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Wed, 09 Jan 2013 10:55:34 +0800
Cc: Ben Myers <bpm@xxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <50EC37FE.4010009@xxxxxxxxx>
References: <50E8F470.5020305@xxxxxxxxxx> <20130107204957.GT27055@xxxxxxx> <50EBC8E5.9080200@xxxxxxxxxx> <50EC37FE.4010009@xxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121028 Thunderbird/16.0.2
On 01/08/2013 11:15 PM, Abhijit Pawar wrote:
> Hi Jeff,
> On 01/08/2013 12:51 PM, Jeff Liu wrote:
>> Hi Ben and Abhijit,
>> On 01/08/2013 04:49 AM, Ben Myers wrote:
>>> Hey Jeff & Abhijit,
>>> On Sun, Jan 06, 2013 at 11:50:08AM +0800, Jeff Liu wrote:
>>>> This small patch convert xfs_parseargs() and suffix_strtoul() to
>>>> use kstrtoint() instead of simple_strtoul() which is deprecated.
>>> It looks like Jeff's patch will conflict with a patch from Abhijit Pawar, 
>>> 'fs:
>>> remove obsolete simple_strto<foo>' which we saw Dec 7.  You guys went after 
>>> the
>>> same thing and came up with slightly differing results.  If Abhijit would
>>> prefer to factor out the xfs changes I'd be happy to take either that, or
>>> Jeff's patch, but it would be better if they didn't conflict.  Can y'all 
>>> come
>>> to an agreement as to what the xfs changes should be?
>> Sorry, I missed that patch from Abhijit.
>> Looks the previous patch did not replaced the simple_strtoul() with
>> kstrtoint() in suffix_strtoul().
>> Abhijit, would you like to fix it as well?
> Yes,it did not replace the call. Looks like I have missed that piece of
> code. Once Ben applies my original patch, I will send another patch for
> this on top of that.
Btw, on the original patch for XFS, you return a negative value per
failure for all those changes in xfs_parseargs(),

+                       rc = kstrtoint(value, 10, &dswidth);
+                       if (rc)
+                               return -EINVAL;

However, it should be EINVAL because xfs_fs_fill_super() will end up
convert it to be negative.

FYI, if we make it to "return EINVAL", checkpatch.pl will show warning
message.  That's why I didn't check up if kstrtoint() failed or not at
this stage on my post, but just proceed to...and those values will be
verified at the end of xfs_parseargs().


<Prev in Thread] Current Thread [Next in Thread>