Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary null check in generated code #59

Merged
merged 1 commit into from
Dec 6, 2017

Conversation

pepijnve
Copy link
Contributor

@pepijnve pepijnve commented Dec 6, 2017

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
Copy link

Can one of the admins verify this patch?

@kofemann
Copy link
Member

kofemann commented Dec 6, 2017

ok to test

2 similar comments
@kofemann
Copy link
Member

kofemann commented Dec 6, 2017

ok to test

@kofemann
Copy link
Member

kofemann commented Dec 6, 2017

ok to test

@kofemann
Copy link
Member

kofemann commented Dec 6, 2017

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

@pepijnve
Copy link
Contributor Author

pepijnve commented Dec 6, 2017

@kofemann done

@dcache-ci
Copy link

Build finished.

@kofemann
Copy link
Member

kofemann commented Dec 6, 2017

retest this please

@kofemann
Copy link
Member

kofemann commented Dec 6, 2017

Thanks!

@kofemann kofemann merged commit 9659944 into dCache:master Dec 6, 2017
@kofemann
Copy link
Member

kofemann commented Dec 6, 2017

btw, do you implement nfs server?

@pepijnve
Copy link
Contributor Author

pepijnve commented Dec 6, 2017

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

@kofemann
Copy link
Member

kofemann commented Dec 6, 2017 via email

@pepijnve
Copy link
Contributor Author

pepijnve commented Dec 6, 2017

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
Copy link
Member

kofemann commented Dec 6, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants