On Wed, 2011-05-25 at 13:30 -0700, Kevin Hilman wrote:
> Tomi Valkeinen <tomi.valkeinen@xxxxxx> writes:
>
> > On Wed, 2011-05-25 at 11:34 -0700, Kevin Hilman wrote:
> >> Tomi Valkeinen <tomi.valkeinen@xxxxxx> writes:
<snip>
> >>
> >> > + if (off_mode_enabled) {
> >> > + count = (count + 1) & 0x7fffffff;
> >> > + dummy_context_loss_counter = count;
> >> > + }
> >>
> >> Again, I don't think this masking is needed. count is already an
> >> 'int', so when it gets bigger than INT_MAX, it will wrap.
> >
> > When count is INT_MAX and one is added to it, it'll wrap to INT_MIN,
> > i.e. maximum negative value, which would be an error value. So by
> > masking out the highest bit we'll get nonnegative count range from 0 to
> > INT_MAX.
> >
> > Perhaps a comment would be justified here =).
>
> Indeed, and using INT_MAX instead of the hard-coded constants would help
> readability also.
It may be just me, but as I see it, INT_MAX is a number like any other,
and using it as a mask feels confusing to me.
Would this be ok to you:
/*
* Context loss count has to be a non-negative value. Clear the sign
* bit to get a value range from 0 to INT_MAX.
*/
count &= ~(1 << 31);
Tomi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
