Juergen Schoenwaelder wrote:
On Wed, Aug 27, 2008 at 08:39:20AM +0200, Arndt Jonasson wrote:
In the error reporting function printError in error.c is this code:
if ((errors[i].level <= smiHandle->errorLevel) && (parser->flags & SMI_FLAG_ERRORS) && ((smiDepth == 1) || (parser->flags & SMI_FLAG_RECURSIVE))) { smiVasprintf(&buffer, errors[i].fmt, ap); (smiHandle->errorHandler) (parser->path, line, errors[i].level, buffer, errors[i].tag); }
The test for smiDepth means that when a module A imports from another module B, errors while reading B are not reported. This may be reasonable for harmless warnings, but if then B imports from a third module C, and this module C isn't found, everything fails, probably with a segmentation violation, and the user gets no indication of why.
Crashes with no reason given are probably the most annoying thing about libsmi. As a side note, the second most annoying is probably SMIv1 MIBs that have been converted to SMIv2 (usually to tidy up because the author used a mixture of v1 and v2 IMPORTS) and generate hundreds of errors.
I think the test (smiDepth == 1)
should be replaced with (smiDepth == 1 || errors[i].level <= 2)
This change probably makes sense from a user's point of view. Of course, the choice of reporting all errors up to and including error level 2 is somewhat arbitrary. Take the example of a company where a core module mixes SMIv1 and SMIv2 and then you get this reported for every company module you compile. But then again, there are ways to redefine error levels so perhaps <= 2 is a reasonable choice.
Anybody having a problem with changing libsmi such that level 0-2 errors are always reported for imported modules?
I'd like it to use whatever is the highest level where parsing could fail, thus making it probable that the process wil core dump later on.