On Feb 16, 2012, at 3:34 PM, Myklebust, Trond wrote:
> On Thu, 2012-02-16 at 15:09 -0500, Chuck Lever wrote:
>> On Feb 16, 2012, at 2:52 PM, Myklebust, Trond wrote:
>>
>>> On Wed, 2012-02-15 at 16:34 -0500, Chuck Lever wrote:
>>>> Eliminate a number of implicit type casts in comparisons, and these
>>>> compiler warnings:
>>>>
>>>> fs/nfs/dir.c: In function ‘nfs_readdir_clear_array’:
>>>> fs/nfs/dir.c:264:16: warning: comparison between signed and unsigned
>>>> integer expressions [-Wsign-compare]
>>>> fs/nfs/dir.c: In function ‘nfs_readdir_search_for_cookie’:
>>>> fs/nfs/dir.c:352:16: warning: comparison between signed and unsigned
>>>> integer expressions [-Wsign-compare]
>>>> fs/nfs/dir.c: In function ‘nfs_do_filldir’:
>>>> fs/nfs/dir.c:769:38: warning: comparison between signed and unsigned
>>>> integer expressions [-Wsign-compare]
>>>> fs/nfs/dir.c:780:9: warning: comparison between signed and unsigned
>>>> integer expressions [-Wsign-compare]
>>>>
>>>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>>> ---
>>>>
>>>> fs/nfs/dir.c | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>>>> index ac28990..8661e13 100644
>>>> --- a/fs/nfs/dir.c
>>>> +++ b/fs/nfs/dir.c
>>>> @@ -207,7 +207,7 @@ struct nfs_cache_array_entry {
>>>> };
>>>>
>>>> struct nfs_cache_array {
>>>> - unsigned int size;
>>>> + int size;
>>>> int eof_index;
>>>> u64 last_cookie;
>>>> struct nfs_cache_array_entry array[0];
>>>>
>>>
>>> Nope. That's a cop-out: array sizes cannot be negative.
>>
>> That's typically true, but IIRC the array size here can hold a "-1" as a special value.
>
>
> Where do you see that? I'm not finding that in the upstream kernel.
Paging this all back in... "eof_index" can be -1, thus it is an int. "size" is compared with eof_index, with signed loop iterators, and with "current_index," which is a loff_t (also a signed type). Therefore "size" should also be signed. This kind of abuse is quite common in the network code.
The other option is to leave these warnings. The patch is a compromise at best, I agree, but mixed sign comparisons have practical implications, while keeping "size" unsigned because it represents a natural number does not have practical implications.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
