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 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.


> 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