dCache / oncrpc4j

Pure Java implementation of ONCRPC/SUNRPC
Other
30 stars 29 forks source link

Xdr#xdrEncodeByteBuffer flips input buffer before using it #62

Closed pepijnve closed 6 years ago

pepijnve commented 6 years ago

Xdr#xdrEncodeByteBuffer calls Buffer#flip on the input parameter before using it. This is rather surprising since Buffer based APIs normally use the position and limit of a Buffer to communicate which range of data should be used. Calling flip on the buffer makes it impossible to use any other position than 0. Is this intentional or a bug?

kofemann commented 6 years ago

The assumption is that buffer is filled outside and then passed to the encode method, like:

Xdr xdr = ...;
ButeByffer buf = ...;

but.put(....);
xdr. xdrEncodeByteBuffer(buf);

Does it makes sense? To me it works as designed, but documentation is missing of course.

pepijnve commented 6 years ago

Typically you work with Buffer as in WritableByteChannel.html#write. In other words, the caller does the flip and the consumer consumes as many bytes as reported by remaining.

I can get it to work, it's just a bit surprising and makes certain usages impossible.

kofemann commented 6 years ago

Well, if you have a use case, then we can change the behavior. I just check the places where we use it and it's not an issue for us to adjust our code.

kofemann commented 6 years ago

@paulmillar What do you think about it? Shall we change the behavior and if yes, shall we fix it in 3.0 (which I have tagged today)? To it it looks like was a bad design decision to flip in xdrEncodeByteBuffer

paulmillar commented 6 years ago

Sounds OK to me

kofemann commented 6 years ago

@pepijnve Today we have release 3.0.1 which includes desired change.

pepijnve commented 6 years ago

That was fast. Thanks!