smidump -f corba : row creation - patch for index columns
Hi,
I don't know if anyone is using the -f corba functionality of smidump, but as indicated in the source-file dump-corba.c :
/* XXX I think we need to include INDEX columns here XXX */
it is indeed needed (as far as I know and as far as I can find in the JIDM documents, otherwise you can't create rows when an index column is non-accessible or read-only) to add the index columns always to the create_xxx functions in the interface SmiEntryFactory : SNMPMgmt::GenericFactory {
which are used to create rows in tables.
So, in attachment you can find a patch which solves this. (for libsmi-0.3.0).
It's the first time I write some code with libsmi, so I hope it's correct :-).
regards,
Brecht
diff -c -r ../temp/libsmi-0.3.0.orig/tools/dump-corba.c ./tools/dump-corba.c *** ../temp/libsmi-0.3.0.orig/tools/dump-corba.c Fri Nov 9 14:48:11 2001 --- ./tools/dump-corba.c Wed Mar 6 00:37:22 2002 *************** *** 1078,1085 **** --- 1078,1087 ---- static void fprintConstructor(FILE *f, SmiNode *smiNode) { SmiNode *childNode; + SmiNode *indexNode; SmiType *smiType; SmiModule *smiModule; + SmiElement *smiElement; char *idlNodeName; char *idlChildNodeName, *idlChildTypeName; int cnt = 0; *************** *** 1091,1116 **** fprintSegment(f, 2*INDENT, "", 0); fprint(f, "%s create_%s (\n", idlNodeName, idlNodeName);
! /* XXX I think we need to include INDEX columns here XXX */ for (childNode = smiGetFirstChildNode(smiNode); childNode; childNode = smiGetNextChildNode(childNode)) { if ((childNode->nodekind == SMI_NODEKIND_SCALAR || childNode->nodekind == SMI_NODEKIND_COLUMN) && current(childNode->status) && childNode->access == SMI_ACCESS_READ_WRITE) {
! cnt++; ! idlChildNodeName = getIdlNodeName(smiGetNodeModule(childNode)->name, childNode->name); ! smiType = smiGetNodeType(childNode); ! idlChildTypeName = getIdlAnyTypeName(childNode, smiType); ! if (cnt > 1) { fprint(f, ",\n"); } - fprintSegment(f, 3*INDENT, "in ", 0); - fprint(f, "%s %s", idlChildTypeName, idlChildNodeName); } } fprint(f, "\n"); --- 1093,1146 ---- fprintSegment(f, 2*INDENT, "", 0); fprint(f, "%s create_%s (\n", idlNodeName, idlNodeName);
! /* First we include the INDEXes */ ! for (smiElement = smiGetFirstElement(smiNode); smiElement; smiElement = smiGetNextElement(smiElement)) { ! cnt++; ! indexNode = smiGetElementNode(smiElement); ! idlChildNodeName = ! getIdlNodeName(smiGetNodeModule(indexNode)->name, ! indexNode->name); ! smiType = smiGetNodeType(indexNode); ! idlChildTypeName = getIdlAnyTypeName(indexNode, smiType); ! if (cnt > 1) { ! fprint(f, ",\n"); ! } ! fprintSegment(f, 3*INDENT, "in ", 0); ! fprint(f, "%s %s", idlChildTypeName, idlChildNodeName); ! } ! for (childNode = smiGetFirstChildNode(smiNode); childNode; childNode = smiGetNextChildNode(childNode)) { + if ((childNode->nodekind == SMI_NODEKIND_SCALAR || childNode->nodekind == SMI_NODEKIND_COLUMN) && current(childNode->status) && childNode->access == SMI_ACCESS_READ_WRITE) { + int matched = 0;
! /* Test if this column is already used as parameter because it is an index */ ! for (smiElement = smiGetFirstElement(smiNode); smiElement; smiElement = smiGetNextElement(smiElement)) { ! indexNode = smiGetElementNode(smiElement); ! if (!strcmp(indexNode->name, childNode->name)) { ! matched = 1; ! break; // already index column ! } ! } ! ! if (!matched) { ! cnt++; ! idlChildNodeName = getIdlNodeName(smiGetNodeModule(childNode)->name, childNode->name); ! smiType = smiGetNodeType(childNode); ! idlChildTypeName = getIdlAnyTypeName(childNode, smiType); ! if (cnt > 1) { fprint(f, ",\n"); + } + fprintSegment(f, 3*INDENT, "in ", 0); + fprint(f, "%s %s", idlChildTypeName, idlChildNodeName); } } } fprint(f, "\n");
Brecht Vermeulen writes:
Brecht> I don't know if anyone is using the -f corba functionality of Brecht> smidump, but as indicated in the source-file dump-corba.c :
Brecht> /* XXX I think we need to include INDEX columns here XXX */
Brecht> it is indeed needed (as far as I know and as far as I can find Brecht> in the JIDM documents, otherwise you can't create rows when an Brecht> index column is non-accessible or read-only) to add the index Brecht> columns always to the create_xxx functions in the interface Brecht> SmiEntryFactory : SNMPMgmt::GenericFactory {
Brecht> which are used to create rows in tables.
Brecht> So, in attachment you can find a patch which solves this. (for Brecht> libsmi-0.3.0).
Thanks. I do not have the spec handy, but it just makes sense to add all INDEX columns as parameters. I have checked the patch into the CVS. Regarding the usage of the -f corba option: I did not receive many bug reports for the CORBA backend and since it is unlikely that I wrote bug-free code, I doubt that many people use the CORBA backend. In fact, my prime motivation of writing it was to understand how the JIDM translation works (and as part of that exercise, I have send quite a few bug reports to the JIDM folks). And since I found quite a few bugs in the C802 draft version, I was asking myself how many people have ever implemented/used these JIDM translations...
Brecht> It's the first time I write some code with libsmi, so I hope Brecht> it's correct :-).
It looks good to me. I change the string comparison since you can in principle use an INDEX element which is imported from another module which has the same name as one of the table columns (pretty unlikely but legal). Libsmi guarantees you that you get the same SmiNode for the same object and thus one can just compare the pointer values.
Anyway, feel free to send more feedback about the JIDM CORBA backend and I would certainly be interested whether you are planning to use this for any production tools or just for academic purposes.
/js
Thanks. I do not have the spec handy, but it just makes sense to add all INDEX columns as parameters. I have checked the patch into the
Well, I've found a draft version of the spec on the Lucent website (they have a gateway). (the official spec has to be paid http://www.opengroup.org/publications/catalog/c802.htm and the free HTML version is a dangling link. In http://www.opengroup.org/onlinepubs/8349099/toc.htm which I've found via google, I haven't found this though...)
The draft says: For each IDL interface generated for groups/table-entries, one IDL operation is defined. The name of the IDL operation is derived by concatenating the identifier of the interface to the string create_ (in the form of create_<interface_id>() ). The return value of the operations is the type of the IDL interface. In addition to the criteria parameter (which is always defined as the last parameter), the types and names of the other parameters of a operation are derived from the index variables and those column variables with read-write and read-create mode of the corresponding group/table-entry. It is assumed that the IDL interface for a group does not have any index variables.
Anyway, feel free to send more feedback about the JIDM CORBA backend and I would certainly be interested whether you are planning to use this for any production tools or just for academic purposes.
okay, attached you can find a new patch (against libsmi0.3.0) which includes the previous one and contains also a patch to add INDEX columns which are imported from other MIBs to the importList, so that a typedef can be added for them. Problem was with the DiffServ MIB (v16): for the DataPathTable, ifIndex (type InterfaceIndex) is an INDEX (imported from IF-MIB), but its type wasn't typedef'ed in IDL (and is used now in the 'create' functions in the SmiEntryFactory interface).
I've also added the extra check : if (smiNode->indexkind == SMI_INDEX_INDEX || smiNode->indexkind == SMI_INDEX_REORDER) { in fprintConstructor before running through the INDEXes.
I don't know if this is required (I'm not an expert in MIBs), but I copied it from fprintRowInterface.
I'm just using it for converting the DiffServ MIB to IDL because I'm trying to manage DiffServ domains with CORBA (please don't shoot me for this :-) ) and wanted to use the already-done work on creating a model for a DiffServ router. And yes, it's for academic purposes (a PhD). And that conversion looks good now. I'll send other patches if necessary. Thanks for the great tool.
regards, Brecht
diff -c -r ./temp_orig/libsmi-0.3.0.orig/tools/dump-corba.c ./libsmi-0.3.0/tools/dump-corba.c *** ./temp_orig/libsmi-0.3.0.orig/tools/dump-corba.c Fri Nov 9 14:48:11 2001 --- ./libsmi-0.3.0/tools/dump-corba.c Wed Mar 6 16:06:06 2002 *************** *** 529,540 **** SmiNode *smiNode; SmiType *smiType; SmiModule *smiTypeModule; ! SmiNodekind kind = SMI_NODEKIND_SCALAR | SMI_NODEKIND_COLUMN;
for (smiNode = smiGetFirstNode(smiModule, kind); smiNode; smiNode = smiGetNextNode(smiNode, kind)) { ! smiType = smiGetNodeType(smiNode); if (smiType) { smiTypeModule = smiGetTypeModule(smiType); --- 529,566 ---- SmiNode *smiNode; SmiType *smiType; SmiModule *smiTypeModule; ! SmiNodekind kind = SMI_NODEKIND_SCALAR | SMI_NODEKIND_COLUMN | SMI_NODEKIND_ROW; ! SmiNode *smiNodeIndex; ! SmiElement *smiElement; ! int j;
for (smiNode = smiGetFirstNode(smiModule, kind); smiNode; smiNode = smiGetNextNode(smiNode, kind)) { ! switch(smiNode->nodekind) { ! case SMI_NODEKIND_ROW: /* Test also if Indexes of tables have to be imported */ ! for (j = 0, smiElement = smiGetFirstElement(smiNode); ! smiElement; ! j++, smiElement = smiGetNextElement(smiElement)) { ! smiNodeIndex = smiGetElementNode(smiElement); ! smiType = smiGetNodeType(smiNodeIndex); ! if (smiType) { ! smiTypeModule = smiGetTypeModule(smiType); ! if (smiTypeModule && ! strcmp(smiTypeModule->name, smiModule->name)) { ! if (strlen(smiTypeModule->name)) { ! addImport(smiTypeModule->name, smiType->name); ! } ! } ! if (smiType->basetype == SMI_BASETYPE_INTEGER32) { ! addImport("SNMPv2-SMI", "Integer32"); ! } ! } ! ! } ! break; ! case SMI_NODEKIND_SCALAR: ! case SMI_NODEKIND_COLUMN: smiType = smiGetNodeType(smiNode); if (smiType) { smiTypeModule = smiGetTypeModule(smiType); *************** *** 548,553 **** --- 574,581 ---- addImport("SNMPv2-SMI", "Integer32"); } } + break; + } } }
*************** *** 1078,1085 **** --- 1106,1115 ---- static void fprintConstructor(FILE *f, SmiNode *smiNode) { SmiNode *childNode; + SmiNode *indexNode; SmiType *smiType; SmiModule *smiModule; + SmiElement *smiElement; char *idlNodeName; char *idlChildNodeName, *idlChildTypeName; int cnt = 0; *************** *** 1091,1116 **** fprintSegment(f, 2*INDENT, "", 0); fprint(f, "%s create_%s (\n", idlNodeName, idlNodeName);
! /* XXX I think we need to include INDEX columns here XXX */ for (childNode = smiGetFirstChildNode(smiNode); childNode; childNode = smiGetNextChildNode(childNode)) { if ((childNode->nodekind == SMI_NODEKIND_SCALAR || childNode->nodekind == SMI_NODEKIND_COLUMN) && current(childNode->status) && childNode->access == SMI_ACCESS_READ_WRITE) {
! cnt++; ! idlChildNodeName = getIdlNodeName(smiGetNodeModule(childNode)->name, childNode->name); ! smiType = smiGetNodeType(childNode); ! idlChildTypeName = getIdlAnyTypeName(childNode, smiType); ! if (cnt > 1) { fprint(f, ",\n"); } - fprintSegment(f, 3*INDENT, "in ", 0); - fprint(f, "%s %s", idlChildTypeName, idlChildNodeName); } } fprint(f, "\n"); --- 1121,1182 ---- fprintSegment(f, 2*INDENT, "", 0); fprint(f, "%s create_%s (\n", idlNodeName, idlNodeName);
! /* First we include the INDEXes */ ! if (smiNode->indexkind == SMI_INDEX_INDEX ! || smiNode->indexkind == SMI_INDEX_REORDER) { ! for (smiElement = smiGetFirstElement(smiNode); smiElement; smiElement = smiGetNextElement(smiElement)) { ! cnt++; ! indexNode = smiGetElementNode(smiElement); ! idlChildNodeName = ! getIdlNodeName(smiGetNodeModule(indexNode)->name, ! indexNode->name); ! smiType = smiGetNodeType(indexNode); ! idlChildTypeName = getIdlAnyTypeName(indexNode, smiType); ! if (cnt > 1) { ! fprint(f, ",\n"); ! } ! fprintSegment(f, 3*INDENT, "in ", 0); ! fprint(f, "%s %s", idlChildTypeName, idlChildNodeName); ! } ! } ! for (childNode = smiGetFirstChildNode(smiNode); childNode; childNode = smiGetNextChildNode(childNode)) { + if ((childNode->nodekind == SMI_NODEKIND_SCALAR || childNode->nodekind == SMI_NODEKIND_COLUMN) && current(childNode->status) && childNode->access == SMI_ACCESS_READ_WRITE) { + int matched = 0;
! /* Test if this column is already used as parameter because it is an index */ ! if (smiNode->indexkind == SMI_INDEX_INDEX ! || smiNode->indexkind == SMI_INDEX_REORDER) { ! for (smiElement = smiGetFirstElement(smiNode); ! smiElement; ! smiElement = smiGetNextElement(smiElement)) { ! indexNode = smiGetElementNode(smiElement); ! if (!strcmp(indexNode->name, childNode->name)) { ! matched = 1; ! break; // already index column ! } ! } ! } ! ! if (!matched) { ! cnt++; ! idlChildNodeName = getIdlNodeName(smiGetNodeModule(childNode)->name, childNode->name); ! smiType = smiGetNodeType(childNode); ! idlChildTypeName = getIdlAnyTypeName(childNode, smiType); ! if (cnt > 1) { fprint(f, ",\n"); + } + fprintSegment(f, 3*INDENT, "in ", 0); + fprint(f, "%s %s", idlChildTypeName, idlChildNodeName); } } } fprint(f, "\n");
Brecht Vermeulen writes:
Brecht> okay, attached you can find a new patch (against libsmi0.3.0) Brecht> which includes the previous one and contains also a patch to Brecht> add INDEX columns which are imported from other MIBs to the Brecht> importList, so that a typedef can be added for them.
Makes sense. I have integrated the patch and checked it into the CVS.
Brecht> I'm just using it for converting the DiffServ MIB to IDL Brecht> because I'm trying to manage DiffServ domains with CORBA Brecht> (please don't shoot me for this :-) ) and wanted to use the Brecht> already-done work on creating a model for a DiffServ Brecht> router. And yes, it's for academic purposes (a PhD).
At least someone using the CORBA driver - that is after all good news. ;-)
/js
participants (2)
-
Brecht Vermeulen -
Juergen Schoenwaelder