Re: RFC on enforcing best-practice through wrappers? [PR15819, PR15722]

[Alexandre Oliva <aoliva at redhat dot com>]

On Nov  5, 2014, Rich Felker <dalias@libc.org> wrote:

> On Wed, Nov 05, 2014 at 04:45:21PM +0000, Joseph Myers wrote:
>> On Wed, 5 Nov 2014, Alexandre Oliva wrote:
>> 
>> > +      struct timeval __now;
>> > +      (void) __gettimeofday (&__now, NULL);
>> > +
>> > +      long int __end = __now.tv_sec * 1000 + __timeout
>> > +	+ (__now.tv_usec + 500) / 1000;
>> 
>> This multiplication by 1000 will overflow on 32-bit systems.  Things may 
>> in fact work OK if it wraps around, but it's not a good idea to rely on 
>> that without explicit casts to unsigned and comments about why it's OK 
>> even when things wrap around.  (Pre-existing condition in the code you're 
>> moving.)

> I suspect it's also semantically wrong. For emulating relative
> timeouts you generally want the monotonic clock, not the realtime
> clock.

Yup.  I suppose I might thank you both your for attentiveness to these
details, even though both issues had been noticed by yours truly.
However, since they were in code being moved by this patchlet, best
practice dictates that it be fixed in separate patches.

Besides, what my message called for was policy discussion, not review of
patchlets that were definitely not intended for inclusion in the
sourcebase yet.

So, anyone got comments on the policy issues, so that the patches can be
both finished and submitted for review, so that the bugs can be
ultimately fixed?

Thanks in advance,

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer