SNMP.pm: SNMP::* package variable behavior

[Bradford Ritchie]

Hi,
  
I'm using the SNMP.pm module version=5.2.1.2, and I have a question/proposal regarding the way the package variables (eg. $SNMP::use_enums) are handled with respect to their corresponding session subroutine parameters in &new() and &update().  Ordinarily I would expect the session creation/update parameter to override the package variable regardless of its value, so that calling "new SNMP::Session( UseEnums => 0, ... )" disables enum translation even when $SNMP::use_enums had previously been set to non-zero.  However, this isn't the case... the session parameters only override the package variables when they are being enabled (from zero to non-zero), but not disabled (non-zero back to zero).

The code that does this (&new and &update) is using the value of the parameter and not the presence of the parameter itself to determine whether or not to copy the corresponding package variable into the session's object data:

   %$this = @_;
<snip>
   $this->{UseLongNames} ||= $SNMP::use_long_names;
   $this->{UseSprintValue} ||= $SNMP::use_sprint_value;
   $this->{BestGuess} ||= $SNMP::best_guess;
   $this->{UseEnums} ||= $SNMP::use_enums;
   $this->{UseNumeric} ||= $SNMP::use_numeric;

Instead, I would think that the code should check for the presence of a parameter (which has already been applied in the @_ assignment), and assign the corresponding $SNMP:: package variable if the parameter is missing.  There are two places where this is done...

In &new(), the session hash keys don't exist yet and are either created from the @_ assignment or from the $SNMP::<var> assignment:

   $this->{UseLongNames} = $SNMP::use_long_names unless defined($this->{UseLongNames});
   $this->{UseSprintValue} = $SNMP::use_sprint_value unless defined($this->{UseSprintValue});
   $this->{BestGuess} = $SNMP::best_guess unless defined($this->{BestGuess});
   $this->{UseEnums} = $SNMP::use_enums unless defined  unless(defined $this->{UseEnums});
   $this->{UseNumeric} = $SNMP::use_numeric unless defined($this->{UseNumeric});

In &update(), the @_ parameters are converted to the %new_fields hash and applied to the session (%this).  So the code is almost the same except that the test is on %new_fields:

   my %new_fields = @_;

   @$this{keys %new_fields} = values %new_fields;

   $this->{UseLongNames} = $SNMP::use_long_names unless defined($new_fields->{UseLongNames});
   $this->{UseSprintValue} = $SNMP::use_sprint_value unless defined($new_fields->{UseSprintValue});
   $this->{BestGuess} = $SNMP::best_guess unless defined($new_fields->{BestGuess});
   $this->{UseEnums} = $SNMP::use_enums unless defined  unless(defined $new_fields->{UseEnums});
   $this->{UseNumeric} = $SNMP::use_numeric unless defined($new_fields->{UseNumeric});

There's also the following code which seems like it would need to be changed:
   # Force UseLongNames if UseNumeric is in use.
   $this->{UseLongNames}++  if $this->{UseNumeric};

...perhaps to this to keep the two variables in sync:
   # Force UseLongNames if UseNumeric is in use.
   $this->{UseLongNames} = $this->{UseNumeric};

Does this seem reasonable?  This is my first attempt at this so please let me know if I'm out of line. 

Thanks.
-- Brad