Oh, this explains why I was forced to spend days (elapsed time weeks)
understanding the Net-SNMP agent code, which I need to modify in order
to implement an audit log feature.

AARRRGGGGHHHHH!!!!!!

A while ago, I posted to the Net-SNMP users mailing list a question
about a guide to the source code, or a design document, which explains
the higher level organization of the code and its flow of control.  No
one answered me.

Now I am flabbergasted that anything leading toward such documentation
has been censored from Net-SNMP commits.

Contrast this policy with the Linux kernel one of retaining comments and
even having a Documentation subdirectory.

                                            --- Omer



On Fri, 2010-05-21 at 19:04 -0400, Doug Manley wrote:
> Hi, team!
> 
> I've submitted numerous patches to the net-snmp project for errors so
> esoteric that it took days to find and understand them.
> 
> In my painful searches through the commentless code, I built up an
> understanding of the necessary components and added in many comments
> to help steer people in the right direction, save them hours of time,
> and help everyone extend the code.
> 
> I recently realized that a patch that I submitted a year or two ago
> had been stripped of all of its comments (and thus a good deal of its
> usefulness).  Granted, my fix (to prevent infinite looping) was kept,
> but all of the knowledge and reasoning behind that fix was stripped.
> 
> Here's a piece the original patch:
> @@ -590,10 +591,24 @@
>  {
>      netsnmp_pdu    *newpdu;
> 
> -    if ((pdu->command != SNMP_MSG_RESPONSE)
> +    if (
> +        // We're only going to fix a PDU if it was a *response* to us.
> +        (pdu->command != SNMP_MSG_RESPONSE)
> +        // We're only going to fix a PDU that had an error.
>          || (pdu->errstat == SNMP_ERR_NOERROR)
> +        // We're only going to fix a PDU that has varbinds.
>          || (0 == pdu->variables)
> -        || (pdu->errindex <= 0)) {
> +        // We're only going to fix a PDU with a positive error-index.
> +        || (pdu->errindex <= 0)
> +        // We're only going to fix a PDU if we can actually remove
> +        // the varbind that was listed as being "bad".  If the number
> +        // of varbinds is *less* than the error-index value, then
> +        // we can't really do anything except simply resend the PDU
> +        // and hope for a better response, and that's not a good
> +        // thing to do; previously, this had led to infinite loops
> +        // of send, receive, and resend.
> +        || (pdu->errindex > snmp_varbind_len(pdu))
> +    ) {
>          return 0;               /* pre-condition tests fail */
>      }
> 
> You'll note that most of the "+" signs are comments.  To someone who's
> trying to find the cause of a problem, these are gold; to that person,
> these variables and conditions have no meaning.  If you haven't worked
> in the code for long periods of time, a lot of the "obvious" things
> that snmp-gurus might know are completely new and foreign to the rest
> of us.
> 
> Maybe someone was against the C++-style comments ("//" comments);
> however, C99 has included them in the C language (see section 6.4.9,
> "Comments", item #2).
> 
> In any case, I've seen this kind of behavior in many open-source
> projects, and it only hurts the project.  If we cannot justify (to our
> readers, to our developers, to our helpers in the open-source
> community), then we're back to square one of trying to control the
> knowledge about the code.
> 
> Unless there is some huge issue of which I am unaware, I strongly
> suggest that everyone fully comment his code; justify its existence,
> and make note of its shortcomings.  In the end, we all benefit.
> 
> Thank you for your time,
> Doug
-- 
In civilized societies, captions are as important in movies as
soundtracks, professional photography and expert editing.
My own blog is at http://www.zak.co.il/tddpirate/

My opinions, as expressed in this E-mail message, are mine alone.
They do not represent the official policy of any organization with which
I may be affiliated in any way.
WARNING TO SPAMMERS:  at http://www.zak.co.il/spamwarning.html


------------------------------------------------------------------------------

_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders

Reply via email to