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

clean up logging statements #241

Open
mfrey opened this issue May 16, 2018 · 1 comment
Open

clean up logging statements #241

mfrey opened this issue May 16, 2018 · 1 comment
Assignees
Labels

Comments

@mfrey
Copy link
Collaborator

mfrey commented May 16, 2018

We should improve the quality of the code of the ccn-lite logging mechanism. While browsing the code, I often read code like the following

 46     char s[CCNL_MAX_PREFIX_SIZE];
 47     (void) s;
 48 
 49     DEBUGMSG_CORE(TRACE, "ccnl_content_new %p <%s [%d]>\n",
 50              (void*) *pkt, ccnl_prefix_to_str((*pkt)->pfx, s, CCNL_MAX_PREFIX_SIZE),
 51              ((*pkt)->pfx->chunknum)? *((*pkt)->pfx->chunknum) : -1);

Every time I'm puzzled for a few seconds why somebody is creating an array and casting it in the next line to void. I assume the array created in line 46 is cast to void in line 47 in order to circumvent a compiler warning about an unused variable. The warning is probably triggered when the code is compiled with logging disabled or set to a higher level than TRACE (in this example).

One solution is to create "boiler plate" code which checks the log level beforehand, e.g.

 45     if (debug_level >= TRACE ) {
 46          char s[CCNL_MAX_PREFIX_SIZE];
 47     
 48          DEBUGMSG_CORE(TRACE, "ccnl_content_new %p <%s [%d]>\n",
 49                   (void*) *pkt, ccnl_prefix_to_str((*pkt)->pfx, s, CCNL_MAX_PREFIX_SIZE),
 50                   ((*pkt)->pfx->chunknum)? *((*pkt)->pfx->chunknum) : -1);
 51     }

The downside (besides the boiler plate) is that the debug_level variable is not thread-safe which is at this point is not a concern, but could cause problems in versions of CCN-lite which should be thread-safe (a discussion we've already had).

Another option would be to put these kind of debug statements into an extra function (which would reduce the boiler plate code). Question is if there are enough DEBUGMSG statements which look like the above (or other examples)?

Any thoughts about it?

@blacksheeep
Copy link
Contributor

I think for those specific cases, casting to void might be the least problematic solution for now?

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

No branches or pull requests

3 participants