dbLoadXxx expand macros in comment lines

Bug #541119 reported by Andrew Johnson
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
EPICS Base
Won't Fix
Medium
Andrew Johnson

Bug Description

From Mark Rivers:

I think there is a bug in the current versions of dbLoadRecords, dbLoadDatabase and dbLoadTemplate. If there is a string that looks like a macro argument in a comment, but the macro is not defined, the parser incorrectly flags an error. The parser should not attempt macro expansion in comments.

Additional information:
Here is a test database, test.db.
#######################
# This is a database with a string that looks like a macro parameter
# in a comment. The string is $(P2).
grecord(ao, $(P1))
{
}
#######################

Here is what happens with dbLoadRecords:
dbLoadRecords("test.db", "P1=mytest")
filename="../../../src/libCom/macLib/macCore.c" line number=709
<string>: P2 referenced but undefined
filename="../dbLexRoutines.c" line number=316
macExpandString failed for file test.db

Here is what happens with dbLoadDatabase:
dbLoadDatabase("test.db","", "P1=mytest")
filename="../../../src/libCom/macLib/macCore.c" line number=709
<string>: P2 referenced but undefined
filename="../dbLexRoutines.c" line number=316
macExpandString failed for file test.db

dbLoadTemplate also gives an error.

Original Mantis Bug: mantis-38
    http://www.aps.anl.gov/epics/mantis/view_bug_page.php?f_id=38

Tags: dbstatic
Revision history for this message
Andrew Johnson (anj) wrote :

This could be a little tricky to fix, as the macro expansion is currently done before we get into the lex code. Luckily we use the lexer to throw away comments rather than the yacc grammar, so it might be do-able that way. The danger is likely to be that some things which used to work won't any more, e.g. using a macro to provide both a field name and value to a record definition. Alternatively we'd have to move the comment stripping into the input buffer management code, which is not nice.

Any other suggestions?

Revision history for this message
Andrew Johnson (anj) wrote :

This problem is too hard to fix in the current lex/yacc code, because we pass input data through the macLib macro expansion routines before it gets anywhere near the parser that identifies and strips out comments. I will try to include a fix in any rewrite I do of the parser in the R3.15 development, but for R3.14 it's not really worth the effort.

Revision history for this message
Ralph Lange (ralph-lange) wrote :

Ping.

We also ran into this. I am setting our internal issue to "won't fix", but we would nevertheless appreciate any step towards fixing this.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

I think everyone has been surprised by this before. I hadn't seen this ticket before though. The complexities of trying to replicate this "pre-processor" behavior in any other way is one of reasons I abandoned my attempt to replace the existing parser. It would mean tokenizing (potentially recursive) macros, which is complicated but doable. It would also be redundant to what macLib already does, but neither code could replace the other. This opens the door to inconsistencies.

While I don't plan on pursuing this myself, the only practical way I can see to deal with this issue is to teach db_yyinput() to strip out comments before macro expansion and parsing (a pre-pre-processor). One way to do this might be to, after fgets(), search the input buffer for '#' and replace it with "\n\0", then ignore input until an actual '\n' is encountered. Long lines will complicate this.

Revision history for this message
Andrew Johnson (anj) wrote :

The startup script could set a variable to suppress the error messages from macLib and just print a warning. Unexpanded macros could still trigger syntax errors in the parser, but the IOC would continue to load if they don't (actually the code already does that, it just prints some scary error messages too). The changes for this are attached.

epics> dbLoadRecords printf-test.db user=anj
macLib: macro bad is undefined (expanding string # $(bad) $(macros)
)
macLib: macro macros is undefined (expanding string # $(bad) $(macros)
)
Warning: 'printf-test.db' line 10 has undefined macros
macLib: macro more is undefined (expanding string # $(more) $(bad) $(macros)
)
macLib: macro bad is undefined (expanding string # $(more) $(bad) $(macros)
)
macLib: macro macros is undefined (expanding string # $(more) $(bad) $(macros)
)
Warning: 'printf-test.db' line 27 has undefined macros
epics> var dbSuppressMacroWarnings 1
epics> dbLoadRecords printf-test.db user=anj
Warning: 'printf-test.db' line 10 has undefined macros
Warning: 'printf-test.db' line 27 has undefined macros
epics>

This code prints a warning for each line in the file that has unexpanded macros. Making that once per file might be possible, but I'll leave someone else to work on that if desired.

Revision history for this message
Andrew Johnson (anj) wrote :

Any comments on the desirability of the dbSuppressMacroWarnings patch I posted for this bug back in December? I'd like to either commit the change (to 3.15) or drop it from my tree. When suppression is turned on it really only makes the warnings less scary; I guess it could have 2 levels so even the new (softer) warning message can be omitted if we think that would be better. The parser will still find any syntax errors in the expanded output, so it's not really dangerous at all.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

From my perspective this change looks innocuous. It doesn't appear to change what is an error, and helpfully adds the line number to the error message.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.