On Tue, 2010-03-30 at 13:07 +0100, Dave Shield wrote:
> On 24 March 2010 07:56, Magnus Fromreide <[email protected]> wrote:
> >> 1) Latch disk statistics:   no / 54x only / both 54x and 52x
> >
> > both -1
> >
> > Adds warnings about silly compares on 32 bit platforms.
> > Adds magic constant instead of using INT32_MAX
> > Works differently on 32 and 64 bit if the multiplication above the
> > comparision overflows.
> >
> > I think the following would be a better pattern to follow for the checks:
> >
> > if (vfs.f_blocks > (INT32_MAX / multiplier))
> >  long_ret = INT32_MAX;
> > else
> >  long_ret = vfs.f_blocks * multiplier;
> 
> 
> In response to these comments, I've drafted a revised version
> of this patch (attached), which uses the comparison code
> structure that you suggest.
>    It also uses INT32_MAX where available (but doesn't rely
> on this necessarily being defined)
> 
> Does this version address your concerns?

Yes - there are only a few nitpicks.

a) It won't compile due to

   if (x < (y / z)

   (note the missing closing parenthesis)

b) I would probably prefer the following since it makes sense to use
   a well known name and this define is located in an implementation
   file.

   #ifndef INT32_MAX
   #define INT32_MAX 0x7fffffff
   #endif

   and yes, I do ignore int's smaller than 32 bits.

/MF


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Net-snmp-coders mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders

Reply via email to