[PATCH 0/8] XDR strings use an unsigned length

[PATCH 0/8] XDR strings use an unsigned length

[Chuck Lever]

Hi Trond, Neil, Bruce...

The following patch series replaces

   [PATCH 06/27] SUNRPC: Fix xdr_decode_string_inplace() argument signage

which I posted last week.

XDR variable length strings use an unsigned length.  This patch series
offers a series of clean ups to make the treatment of file names and path
names in the NFS and NLM server consistent with the XDR specification and
other areas of the NFS client.

-- 
corporate:    <chuck dot lever at oracle dot com>

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/8] XDR strings use an unsigned length

[J. Bruce Fields]

On Thu, Nov 01, 2007 at 04:56:41PM -0400, Chuck Lever wrote:
> Hi Trond, Neil, Bruce...
> 
> The following patch series replaces
> 
>    [PATCH 06/27] SUNRPC: Fix xdr_decode_string_inplace() argument signage
> 
> which I posted last week.
> 
> XDR variable length strings use an unsigned length.  This patch series
> offers a series of clean ups to make the treatment of file names and path
> names in the NFS and NLM server consistent with the XDR specification and
> other areas of the NFS client.

I guess I'm a little worried about these patches.

It's not inherently a bug to store a length in an int.

So if the point is to track down the causes of signed/unsigned
comparison warnings, so that they stand out more in the future and draw
attention to particular bugs, that's fine.  But that's only helpful if
we do actually analyze each such case carefully to determine whether
there are bugs.

So, for example, if we're going to change the nfsd_lookup arguments to
take an unsigned int for a length, I'd be much reassured if that came
with an argument as to what exactly happens currently when we recieve a
"negative" length for the lookup argument off the wire.  Otherwise we're
defeating the purpose of these warnings, aren't we?

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/8] XDR strings use an unsigned length

[Chuck Lever]

On Nov 2, 2007, at 6:06 PM, J. Bruce Fields wrote:
> On Thu, Nov 01, 2007 at 04:56:41PM -0400, Chuck Lever wrote:
>> Hi Trond, Neil, Bruce...
>>
>> The following patch series replaces
>>
>>    [PATCH 06/27] SUNRPC: Fix xdr_decode_string_inplace() argument  
>> signage
>>
>> which I posted last week.
>>
>> XDR variable length strings use an unsigned length.  This patch  
>> series
>> offers a series of clean ups to make the treatment of file names  
>> and path
>> names in the NFS and NLM server consistent with the XDR  
>> specification and
>> other areas of the NFS client.
>
> I guess I'm a little worried about these patches.
>
> It's not inherently a bug to store a length in an int.

Do my patches suggest that it is a bug?  Most of the patch  
descriptions state that this work is a "clean up" effort, done to  
make all the XDR helpers handle object lengths in a way consistent  
with the protocol specifications and each other.

> So if the point is to track down the causes of signed/unsigned
> comparison warnings, so that they stand out more in the future and  
> draw
> attention to particular bugs, that's fine.

The point of this work is to help reduce the likelihood of  
introducing new bugs.  We do that by making the code more clear, by  
introducing specific coded checks about our assumptions, by making it  
possible to use our tool chain more effectively, and by providing  
documentation.

> But that's only helpful if
> we do actually analyze each such case carefully to determine whether
> there are bugs.

If I argue there are no bugs, the simple way to prove me wrong is to  
demonstrate a bug.

> So, for example, if we're going to change the nfsd_lookup arguments to
> take an unsigned int for a length, I'd be much reassured if that came
> with an argument as to what exactly happens currently when we  
> recieve a
> "negative" length for the lookup argument off the wire.

I refer you to protocol specs (they say "the length is always  
unsigned") and last week's thread on this list that discusses this  
very issue.

http://sourceforge.net/mailarchive/forum.php? 
thread_name=20071031165045.5861.52308.stgit%40manray. 
1015granger.net&forum_name=nfs

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs