View Issue Details

IDProjectCategoryView StatusLast Update
0000179ascendgeneralpublic2008-02-10 13:03
Reporterjds 
Assigned Tojohn 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version 
Target VersionFixed in Version0.9.6 
Summary0000179: Non-ansi variadic macros
DescriptionThe 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):

#ifdef __GNUC__
# 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:

instantiate.c:5359:
  CONSOLE_DEBUG("ENTERED ExecuteBlackBoxExt\n");

..\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__); \
           fprintf(stderr,"\n"); \
         }

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.
TagsNo tags attached.

Relationships

Activities

john

2005-12-22 02:35

administrator   ~0000150

I'll take this on. I do currently have 250 calls to error_reporter in the code, so give me a minute!

ben

2005-12-23 10:47

manager   ~0000190

Checking macro
__STRICT_ANSI__
before dropping into gnuc preprocessor blocks
may be an approach to think about.

john

2006-01-25 06:32

administrator   ~0000262

Reminder sent to: jds

Hi Jerry

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.

Cheers
JP

jds

2006-01-25 23:29

manager   ~0000263

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

2006-01-28 06:07

 

error.h (7,241 bytes)

2006-01-28 06:07

 

error.c (6,759 bytes)

jds

2006-01-28 06:19

manager   ~0000264

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.

Thanks, Jerry

john

2006-01-28 06:44

administrator   ~0000265

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

eg
svn diff error.* > error.patch

Cheers
JP

john

2006-02-07 12:29

administrator   ~0000271

Hi Jerry,

This one's OK now, right? See changeset 278 and changeset 276.

john

2006-03-09 06:57

administrator   ~0000296

I think this is fixed now. Please reopen if not.

Issue History

Date Modified Username Field Change
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