On Wed, 20 Jul 2005 17:02:24 +0100 Dave wrote: DS> I've been doing a bit more work with the 'array-user' framework,
Yeah, I noticed the cvs activity and knew what section of the book you were working on. :-) DS> The first relates to the return value of the 'delete_row' DS> routine. This is defined as returning a 'netsnmp_index*', DS> which elsewhere tends to be used for passing the per-row DS> data structure. DS> Judging by the mib2c-generated code, the purpose of this routine DS> is to release the memory used by that data structure. So it probably DS> wouldn't be safe to follow the row pointer afterwards. Yep. And the return value is hard-coded to NULL. I can only guess that maybe originally it was a remove funtion that was re-tooled, and the prototype didn't get changed. It should be void. DS> What should this routine return: DS> a) if deleting the row succeeds ? DS> b) if deleting the row fails ? I looked at how it is used in the helper code, and it doesn't check the return code. It is called during set processing, to delete either the undo or existing row, depending on the error status of the set. DS> The second question concerns the 'all_helpers.h' consolidated DS> header file. That file includes a reference to the 'table_array.h' DS> header file, but this is commented out. Can you remember why? Nope. Wes created the file, and it was commented out when he created it. I can only guess that it caused compile problems at the time... DS> Is there any reason why 'all_helpers.h' shouldn't include DS> this helper along with all of the others? Not that I can think of. DS> I've also noticed some minor inconsistencies between the prototype DS> definitions of some of the hook routines in this header file, and DS> the code generated by mib2c. The hooks are mostly defined using DS> 'netsnmp_index*' parameters (apart from the row_copy and row_compare DS> hooks, which use 'void*') DS> But the mib2c output defines routines using the table-specific DS> context data structure. This mismatch seems to irritate the compiler, DS> unless the relevant hook assignments are explicitly re-cast (as they DS> are in the mib2c output). This was a conscious decision. The prototypes for the hooks, obviously, must use the netsnmp_index, because the hooks are the same for all tables. But it seemed silly to define the functions w/netsnmp_index, and then have to immediately create a local var and cast the parameter. So the generated functions use the actual type for the particular table. If there are missing casts that bug the compiler, they should be fixed. DS> This feels a little risky and/or potentially confusing - is it DS> perhaps worth tweaking the mib2c.array-user.conf file to bring DS> the generated code into line with the header expectations? DS> Obviously, that would need explicit casts of the parameters to DS> get them into a form that could actually be used. But this feels DS> a slightly cleaner approach. I don't agree. I like having the casting happens once, in the hook assignments, instead of in every function. I makes the functions themselves a little easier to understand for the users, and in this case I think that's more important that a cleaner approach (which is arguable, but I won't go there!). DS> One other thought - the generated 'row_copy' routine includes code DS> to duplicate the index OID array. Is this strictly necessary? DS> This routine seems to be used to take a copy of the previous DS> contents of a row, so it can be restored if processing the SET DS> request fails. But this assignment won't affect the index values DS> (or any of the non-writeable column objects come to that). DS> So is there any real need to copy these values across and back? I think all the values must be copied, since the processing of a settable variable could very well result in a change to a read-only variable. As for the indexes, you are correct that the current use by the helper could get away without the indexes. But the function is more generic, and I think a row copy function should copy the entire row. And at this late stage of the game, I'd really rather not change functionality of existing functions. If you'd really like to optimize away an alloc and a memcopy of a few bytes on a failed set, then I'd be ok with a new callback hook (row_data_copy) and generated function (of the same name, which row_copy could call). Of course, then you should also create another callback (duplicate_row_no_index) & corresponding function to save the same alloc/memcopy that happens at the beginning of every set. -- Robert Story; NET-SNMP Junkie Support: <http://www.net-snmp.org/> <irc://irc.freenode.net/#net-snmp> Archive: <http://sourceforge.net/mailarchive/forum.php?forum=net-snmp-coders> You are lost in a twisty maze of little standards, all different. ------------------------------------------------------- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click _______________________________________________ Net-snmp-coders mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
