View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0000179||ascend||general||public||2005-12-21 22:03||2008-02-10 13:03|
|Target Version||Fixed in Version||0.9.6|
|Summary||0000179: Non-ansi variadic macros|
|Description||The variadic macros defined in utilities/error.h cause compiler warnings using gcc with -std=c99. This is using gcc/mingw. Not sure of gcc/linux.|
There are 2 issues with the current implemention, ansi-wise.
1. the macro definitions when __GNUC__ is defined use a gnu extension which is non-ansi and results in a compiler warning in every ASCEND source file (since all include ascConfig.h, which includes error.h):
# define ERROR_REPORTER_DEBUG(MSG,args...) error_reporter(ASC_PROG_NOTE,__FILE__,__LINE__,"%s: " MSG, __func__, ##args)
# define CONSOLE_DEBUG(MSG,args...) fprintf(stderr,"%s:%d (%s): " MSG "\n", __FILE__,__LINE__,__func__, ##args)
../base/generic/utilities/error.h:59:39: warning: ISO C does not permit named variadic macros
../base/generic/utilities/error.h:60:32: warning: ISO C does not permit named variadic macros
2. In ansi, variadic macros require that at least 1 of the variable arguments be actually specified when the macro is used. In the current definition of the macros, a compiler warning results when the macro is used with only a single argument. For example:
..\base\generic\compiler\instantiate.c:5359:47: warning: ISO C99 requires rest arguments to be used
One solution to both these problems is to elminate the named argument:
# define ERROR_REPORTER_DEBUG(...) \
error_reporter(ASC_PROG_NOTE, __FILE__, __LINE__, __VA_ARGS__)
# define CONSOLE_DEBUG(...) \
fprintf(stderr,"%s:%d (%s): ", __FILE__, __LINE__, __func__); \
fprintf(stderr, __VA_ARGS__); \
To keep the __func__ report in ERROR_REPORTER_DEBUG() would require changing the signature and implementation of error_reporter() to treat the function name explicitly (as with __FILE__ and __LINE__):
# define ERROR_REPORTER_DEBUG(...) \
error_reporter(ASC_PROG_NOTE, __FILE__, __LINE__, __func__, __VA_ARGS__)
int error_reporter(const error_severity_t sev,
const char *errfile,
const int errline,
const char *errfunc,
const char *fmt, ...);
We should not bow at the altar of ansi-compliance when necessary functionality happens not to be strict ansi. However, here there appears to be a way to provide the functionality and keep to standard usage, so should be considered.
I am happy to draft a modification, but as this is John's baby, it's up to him.
|Tags||No tags attached.|
||I'll take this on. I do currently have 250 calls to error_reporter in the code, so give me a minute!|
before dropping into gnuc preprocessor blocks
may be an approach to think about.
Reminder sent to: jds
Are you still getting warnings on this one? I think I managed to implement a workaround for GCC and C-99 compilers, but maybe you need to add #else cases for other compilers in error.h. I found that all major compilers support variadic macros, so it's just a question of getting the #define right for that platform.
Yes, the error type #2 is still warned about whenever you use the macro without any variable arguments. This usage is non-standard, so gcc issues the warning.
This is one of only a few remaining warnings from gcc under strict ansi (most are associated with the extfunc mess). Since a standard-compliant solution exists for this warning, it would seem desireable to use it (rather than to macro around compilers that tolerate non-standard usage).
error.h (7,241 bytes)
error.c (6,759 bytes)
Possible fixes to error.[ch] have been attached. These versions are ansi c-99 compliant. The macros in error.h are not as obvious about requiring a MSG argument as before - they do need at least 1 variable argument, the format string - but this could be emphasized in commentary.
In order to keep the __func__ reporting in the macros, I changed the signature of error_reporter(), va_error_reporter(), and the callback function. This will require modifying the handful of direct references to error_reporter() in the code base, but does not appear too monumental a task.
Changing the callback signature may cause unacceptable problems in your python or other calling code. If this is the case, an intermediary function could be created for the macros to call that formats the message with the function name and calls the old version of error_reporter(). This would allow keeping the former signatures but make the macros ansi-compliant.
Also, there are XTERM escape sequences embedded in CONSOLE_DEBUG(). These should be made optional as in error.c.
Please provide feedback. If the changes are acceptable, I can follow through on updating the direct references to error_reporter() and commit the changes. If more work is needed, that's fine, but let's please keep working towards an ansi-compliant approach.
I'll take a look at this now, Jerry. For future reference, a patch is more useful than new files: I can directly see what you're changing with a patch
svn diff error.* > error.patch
This one's OK now, right? See changeset 278 and changeset 276.
||I think this is fixed now. Please reopen if not.|
|2005-12-21 22:03||jds||New Issue|
|2005-12-22 01:57||john||Status||new => assigned|
|2005-12-22 01:57||john||Assigned To||=> john|
|2005-12-22 02:35||john||Note Added: 0000150|
|2005-12-23 10:47||ben||Note Added: 0000190|
|2006-01-25 06:32||john||Note Added: 0000262|
|2006-01-25 23:29||jds||Note Added: 0000263|
|2006-01-28 06:07||jds||File Added: error.h|
|2006-01-28 06:07||jds||File Added: error.c|
|2006-01-28 06:19||jds||Note Added: 0000264|
|2006-01-28 06:44||john||Note Added: 0000265|
|2006-02-07 12:29||john||Note Added: 0000271|
|2006-02-07 13:17||john||Target release||=> 1.0|
|2006-02-07 13:22||john||Target release||1.0 => 0.9.6|
|2006-03-09 06:57||john||Status||assigned => resolved|
|2006-03-09 06:57||john||Resolution||open => fixed|
|2006-03-09 06:57||john||Note Added: 0000296|
|2006-06-22 07:36||john||Status||resolved => feedback|
|2006-06-22 07:41||john||Status||feedback => resolved|
|2006-06-22 07:41||john||Fixed in Version||=> 0.9.6|
|2008-02-10 13:03||john||Status||resolved => closed|