Re: [Libtirpc-devel] [PATCH V3] xdrstdio_create buffers do not output encoded values on ppc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 07/10/2018 11:51 AM, Chuck Lever wrote:
> 
> 
>> On Jul 10, 2018, at 11:43 AM, Steve Dickson <SteveD@xxxxxxxxxx> wrote:
>>
>>
>>
>> On 07/10/2018 10:49 AM, Chuck Lever wrote:
>>>
>>>
>>>> On Jul 10, 2018, at 10:44 AM, Steve Dickson <SteveD@xxxxxxxxxx> wrote:
>>>>
>>>> From: Daniel Sands <dnsands@xxxxxxxxxx>
>>>>
>>>> The cause is that the xdr_putlong uses a long to store the
>>>> converted value, then passes it to fwrite as a byte buffer.
>>>> Only the first 4 bytes are written, which is okay for a LE
>>>> system after byteswapping, but writes all zeroes on BE systems.
>>>>
>>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1261738
>>>>
>>>> Signed-off-by: Steve Dickson <steved@xxxxxxxxxx>
>>>> ---
>>>> v3: Reworked the bounds checking
>>>>
>>>> v2: Added bounds checking
>>>>   Changed from unsigned to signed
>>>>
>>>> src/xdr_stdio.c | 14 +++++++++++---
>>>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/xdr_stdio.c b/src/xdr_stdio.c
>>>> index 4410262..b902acd 100644
>>>> --- a/src/xdr_stdio.c
>>>> +++ b/src/xdr_stdio.c
>>>> @@ -103,10 +103,12 @@ xdrstdio_getlong(xdrs, lp)
>>>> 	XDR *xdrs;
>>>> 	long *lp;
>>>> {
>>>> +	int32_t mycopy;
>>>>
>>>> -	if (fread(lp, sizeof(int32_t), 1, (FILE *)xdrs->x_private) != 1)
>>>> +	if (fread(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs->x_private) != 1)
>>>> 		return (FALSE);
>>>> -	*lp = (long)ntohl((u_int32_t)*lp);
>>>> +
>>>> +	*lp = (long)ntohl(mycopy);
>>>> 	return (TRUE);
>>>> }
>>>>
>>>> @@ -115,8 +117,14 @@ xdrstdio_putlong(xdrs, lp)
>>>> 	XDR *xdrs;
>>>> 	const long *lp;
>>>> {
>>>> -	long mycopy = (long)htonl((u_int32_t)*lp);
>>>> +	int32_t mycopy;
>>>> +
>>>> +#if defined(_LP64)
>>>
>>> Hi Steve, _LP64 is gcc-specific. Hope that's OK.
>>>
>>> Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> Here the reason I left it:
>>   https://stackoverflow.com/questions/685124/how-to-identify-a-64-bit-build-on-linux-using-the-preprocessor
>>
>> Should it be removed?
> 
> Not at all, I'm just pointing out that if libtirpc is built
> with a non-gcc compiler, _LP64 is not guaranteed to be
> defined.
> 
> That stackoverflow article suggests a couple of more portable
> ways of checking for 64-bit.
> 
> Are there distributions that might build libtirpc with a
> non-gcc compiler (like Clang) ? If no, then this isn't an
> issue at all.
Lets cross that bridge if/when it comes... 

Thanks for the time!

steved.
 
> 
> 
>> steved.
>>
>>>
>>>
>>>> +	if ((*lp > INT32_MAX) || (*lp < INT32_MIN))
>>>> +		return (FALSE);
>>>> +#endif
>>>>
>>>> +	mycopy = (int32_t)htonl((int32_t)*lp);
>>>> 	if (fwrite(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs->x_private) != 1)
>>>> 		return (FALSE);
>>>> 	return (TRUE);
>>>> -- 
>>>> 2.17.1
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Check out the vibrant tech community on one of the world's most
>>>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>>>> _______________________________________________
>>>> Libtirpc-devel mailing list
>>>> Libtirpc-devel@xxxxxxxxxxxxxxxxxxxxx
>>>> https://lists.sourceforge.net/lists/listinfo/libtirpc-devel
>>>
>>> --
>>> Chuck Lever
> 
> --
> Chuck Lever
> 
> 
> 
--
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



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux