dCache / oncrpc4j

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

Remove unnecessary null check in generated code #59

Closed pepijnve closed 6 years ago

pepijnve commented 6 years ago

jrpcgen currently generates unwanted null checks when the iterative linked list encoding approach is used. This causes compile errors in the generated code for fields with primitive types. As an example

For Entry3 from the NFSv3 spec

struct Entry3 {
    unsigned hyper      fileid;
    string    name<>;
    unsigned hyper      cookie;
    Entry3       *nextentry;
};

jrpcgen currently generates

public long fileid;
public String name;
public long cookie;
public Entry3 nextentry;

public void xdrEncode(XdrEncodingStream xdr) throws OncRpcException, IOException {
    Entry3 $this = this;
    do {
        if( $this.fileid != null) xdr.xdrEncodeLong($this.fileid);
        if( $this.name != null) xdr.xdrEncodeString($this.name);
        if( $this.cookie != null) xdr.xdrEncodeLong($this.cookie);
        $this = $this.nextentry;
        xdr.xdrEncodeBoolean($this != null);
    } while ( $this != null );
}

This PR removes those null checks. As far as I can tell, codingMethod can be used without the additional check since it already generates code that deals with null values.

dcache-ci commented 6 years ago

Can one of the admins verify this patch?

kofemann commented 6 years ago

ok to test

kofemann commented 6 years ago

ok to test

kofemann commented 6 years ago

ok to test

kofemann commented 6 years ago

@pepijnve Thanks for the pull-request. Can you add "signed-off" to your commit? (git commit --amend -s)

pepijnve commented 6 years ago

@kofemann done

dcache-ci commented 6 years ago

Build finished.

kofemann commented 6 years ago

retest this please

kofemann commented 6 years ago

Thanks!

kofemann commented 6 years ago

btw, do you implement nfs server?

pepijnve commented 6 years ago

No, I'm using the client-side functionality only. Looking into using oncrpc4j for a user space NFSv3 client.

kofemann commented 6 years ago

cool. Have you seen our client ( https://github.com/dCache/nfs4j/tree/master/core/src/main/java/org/dcache/nfs/v4/client) or one from EMC (https://github.com/EMCECS/nfs-client-java).

Out one is a playground staff for testing, but EMC's one looks like for a production.

-kofemann /* caffeinated mutations of the core personality /

On Wed, Dec 6, 2017 at 6:05 PM, Pepijn Van Eeckhoudt < notifications@github.com> wrote:

No, I'm using the client-side functionality only. Looking into using oncrpc4j for a user space NFSv3 client.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dCache/oncrpc4j/pull/59#issuecomment-349707017, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjJ3Tnev8e12msJoDf2oQDxXVbWh60Sks5s9sl2gaJpZM4Q3-ZJ .

pepijnve commented 6 years ago

I had seen the nfs4j client code. That's v4 only though and I need to support v3. I didn't know about the EMC code yet. Thanks for the pointer. I'll look into, but at first glance it seems worthwhile to stick with oncrpc4j as the base RPC client. That seems to be more fleshed out than the EMC equivalent (e.g., RPCGSS support out of the box).

kofemann commented 6 years ago

Well, we have implemented RPCSEC_GSS only for server side (see issue #28). Anyway, we are pleased to see that you have chosen our library to implement your sunrpc based service. Feel free to open issues, if something does not work as expected. And, of course, we hare happy to merge your pull requests :).