|Summary:||ASTERISK-17973: Possible error in variable declaration in /include/asterisk/logger.h|
|Reporter:||Luke H (luckman212)||Labels:|
|Date Opened:||2011-06-07 06:05:23||Date Closed:||2011-09-28 16:20:40|
|Environment:||Attachments:||( 0) logger_h.patch|
|Description:||I was poking around in the '/include/asterisk/logger.h' code and it appears (to my untrained eye at least) that on line 171:|
#define LOG_VERBOSE __LOG_VERBOSE, _A_
..should actually be..
#define AST_LOG_VERBOSE __LOG_VERBOSE, _A_
I say this because that would follow the structure of the other variable declarations. The code block seems to first release AST_LOG_VERBOSE (#undef AST_LOG_VERBOSE) but then instead of redefining it, it defines LOG_VERBOSE which is duplicated -- already defined from the previous clause. I admit I do not really understand the code here so I might be mistaken.
|Comments:||By: Leif Madsen (lmadsen) 2011-06-13 15:27:15.896-0500|
I've removed the inline code. Can you please attach your issue as a patch to this issue and mark it as so? Thanks!
By: Luke H (luckman212) 2011-07-21 19:54:41.790-0500
I have attached the .patch as of 7/27/11. I am still not sure this patch is "valid" but I would like any feedback on its validity from someone who knows the code better than I do.
By: Luke H (luckman212) 2011-09-01 08:25:11.240-0500
Has anyone confirmed or denied this?
By: Walter Doekes (wdoekes) 2011-09-01 13:39:21.432-0500
Although I must say the AST_* macro aliases should probably be removed instead, seeing the usage pattern:
~/src/asterisk-1.8.x$ grep AST_LOG include/asterisk/logger.h | sed -e 's/^[^ ]* //;s/ .*//;/AST_LOG/!d;s/^AST_//' | sort | uniq | while read x ; do echo -n "AST_$x: " ; find . -name '*.c' -or -name '*.h' | xargs grep "AST_$x" | wc -l ; echo -n "$x: " ; find . -name '*.c' -or -name '*.h' | xargs grep "[^_]$x" | wc -l ; done | sort -n
By: Luke H (luckman212) 2011-09-01 13:44:09.856-0500
Wow, that's some impressive command-line-fu! I am not supposed to commit this myself, right? I mean, I don't have commit access. So I assume the "Ship it!" was directed to the powers that be.