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

Recheck source code in coap_dtls_new_context() #293

Open
elfring opened this issue Jan 26, 2019 · 8 comments
Open

Recheck source code in coap_dtls_new_context() #293

elfring opened this issue Jan 26, 2019 · 8 comments

Comments

@elfring
Copy link

elfring commented Jan 26, 2019

The implementation of the function “coap_dtls_new_context” does not contain a goto statement. The jump label “fail” can not be reached.
How do you think about to adjust (or delete) a bit of source code here?

@obgm
Copy link
Owner

obgm commented Jan 26, 2019

Unfortunately, it does contain a goto failstatement. I would like to get rid of that side-effect of the macro G_CHECK, i.e., make the jump obvious.

@oliness
Copy link

oliness commented Jan 30, 2019

@obgm Wouldn't removing the goto fail statement from G_CHECK lead to lots of duplicate code as you'd have to put in if ... goto fail each time. Wouldn't it it be better to rename the macro to make it obvious what it does?

How about "G_RET_ZERO_OR_FAIL"?

@obgm
Copy link
Owner

obgm commented Jan 30, 2019

It is a macro, so this is duplicate source code anyway. But I agree that renaming would be better than nothing. @mrdeep1 do you have an opinion on this?

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Jan 30, 2019

The issue with a long macro name is that there is likelihood of exceeding the line length of 80 characters.
[Anything longer than G_CHECK will require lines to be split in most cases in the current code]

Many of the GnuTLS examples make the use of just CHECK (which normally triggers and assert()), I decided to prefix with G_ to indicate this was a CHECK for GnuTLS.

That said, I am all for better readability of the code making it self documenting where possible. To that end, I would prefer something like G_CHECK_FAIL.

@obgm
Copy link
Owner

obgm commented Jan 30, 2019

Sounds good.

@oliness
Copy link

oliness commented Jan 30, 2019

Ok is everyone fine with changing G_CHECK to G_CHECK_FAIL? If so I'll submit a pr, ensuring lines are split when needed.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Jan 30, 2019

@oliness If you are happy to do the work, that is fine by me.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Feb 20, 2019

@oliness Are you going to be able to do this work?

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

No branches or pull requests

4 participants