On 23 June 2010 21:11, Magnus Fromreide <ma...@lysator.liu.se> wrote:
> I think r17794 fixed this issue.

As far as I can tell, revision 17794 is purely concerned with how
ranges are *displayed*  as part of dumping MIB structures.
This doesn't seem to affect how the values are actually handled internally.

The technique used here (casting the signed int buffer to act as an
unsigned buffer) would probably work OK elsewhere, but it's not
clear to me that this is currently being done.

For example, in the routine 'parse.c:parse_ranges()', the MIB
definition is parsed as follows:

    low = strtoul(nexttoken, NULL, 10);
    high = strtoul(nexttoken, NULL, 10);

(So these values are parsed as unsigned integers, which might cause
problems for a range involving negative values ....)

but then

   (*rpp)->low = low;
   (*rpp)->high = high;

so these (unsigned) values are stored into signed fields, which would
cause problems for values between 2^31 and 2^32-1 on a 32-bit system.

The one place where this range seems to be used is 'snmp_api.c:_check_range()'
which is passed a signed long value, and checks this against the signed
range values.   Again, this probably won't work with values  2^31..2^32-1



>> The fix is simple, just use long instead of int. But wouldn't it be ABI
>> breaker? Changing sizes of structures in public headers is bad!

Two comments.
Firstly, I would regard  'range_list'  as an internal data structure,
rather than part of the public API, so it would be legitimate (IMO)
to tweak this as part of a new major release.
    Hence it might be possible to make such a change for 5.6 (if we move fast)

However, would this actually help?   On 32-bit systems, 'long' and 'int' are
typically both 32 bits, so suffer from exactly the same issues.
The problem (as I see it) is one of signed vs unsigned, rather than 32
vs 64 bit.


I suspect that what is probably needed is some form of flag within the
range_list structure, to indicate whether the values should be treated as
signed or unsigned (with appropriate casts if necessary).



> Trunk seems to work (x86_64, x86 on a 64-bit kernel (also V5-5-patches)
> and Solaris 10)

If you're testing this on a 64-bit system, then yes - things will probably work
OK, since the 32-bit ranges (both signed and unsigned) will fit comfortably
within a 64-bit signed variable.
   The problems only start to bite when you're working with a 32-bit OS,
and using values in the overflow range.   (2^31..2^32-1, or <0)


Dave

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate 
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the 
lucky parental unit.  See the prize list and enter to win: 
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders

Reply via email to