Error reporting with nested imports
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.
I think the test (smiDepth == 1)
should be replaced with (smiDepth == 1 || errors[i].level <= 2)
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.
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?
/js
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.
On Wed, Aug 27, 2008 at 09:48:26PM +1000, Andrew Hood wrote:
Crashes with no reason given are probably the most annoying thing about libsmi.
A segfault is a programming error. If you have examples where libsmi crashes with a segfault, post them here so that we can see what can be done about this.
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.
Converted by smidump? Please send me an example to look at.
/js
Hi,
While at this subject:
If the PATH is not configured correctly, almost all imports fail and the program crashes/segfualts. The user has no hint of what went wrong.
I think that in this case the program should report an error: "import module [module-name] not found" and exit gracefully.
A more user friendly and helpful way to indicate the problem and let the user a chance to fix his/her PATH configuration.
Please let me know what you think.
Thanks,
Yigal
---- Arndt Jonasson arndt@tail-f.com 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.
I think the test (smiDepth == 1)
should be replaced with (smiDepth == 1 || errors[i].level <= 2) -- !! This message is brought to you via the `libsmi' mailing list. !! Please do not reply to this message to unsubscribe. To unsubscribe or adjust !! your settings, send a mail message to libsmi-request@ibr.cs.tu-bs.de !! or look at https://mail.ibr.cs.tu-bs.de/mailman/listinfo/libsmi.
On Wed, Aug 27, 2008 at 07:41:27AM -0400, Yigal Hochberg wrote:
If the PATH is not configured correctly, almost all imports fail and the program crashes/segfualts. The user has no hint of what went wrong.
I think that in this case the program should report an error: "import module [module-name] not found" and exit gracefully.
$ export SMIPATH=/foo $ smilint -s ./IF-MIB ./IF-MIB:6: [1] failed to locate MIB module `SNMPv2-SMI' ./IF-MIB:9: [1] failed to locate MIB module `SNMPv2-TC' ./IF-MIB:11: [1] failed to locate MIB module `SNMPv2-CONF' ./IF-MIB:12: [1] failed to locate MIB module `SNMPv2-MIB' ./IF-MIB:13: [1] failed to locate MIB module `IANAifType-MIB' [...]
It seems to work for me... (0.4.6).
A more user friendly and helpful way to indicate the problem and let the user a chance to fix his/her PATH configuration.
Perhaps additional level 6 messages can be generated at the end of the run. But then the default is not to show level 6 messages. ;-)
/js
participants (4)
-
Andrew Hood -
Arndt Jonasson -
Juergen Schoenwaelder -
Yigal Hochberg