On 8/18/13, patrick keshishian <sids...@boxsoft.com> wrote: > Swaping accounts for sending diff in-line and not have > gmail fuck it up. > > On Sat, Aug 17, 2013 at 20:03:32 -0600, Theo de Raadt wrote: >> > > > So basically, this takes a long long value, and casts it to a long. >> > > >> > > Yes, but that's not the entire story. >> > > >> > > the function declaration is: long nextTimeout(int resolution) >> > > >> > > So it would truncate anyway. But look at it closer. It >> > > takes the value of timeval.tv_sec (your time_t / long long) >> > > and mod's it with resolution (an int value). The only thing >> > > it then does is multiply it by 1000L. >> > > >> > > The only reason this function "broke" is C++'s std::max() >> > > defined as a template function, requiring both args be of >> > > the same typename. Since the original code uses 1000L >> > > literal for the first arg, the compiler was having issues with >> > > the long long as the second arg., but ... >> > > >> > > What is long long % int? another int value? Yes, I realize >> > > it would be a long long according to the compiler, but, >> > > I fail to see where/how 0xFFFFFFFFFFFFFFFF % 0xFF would >> > > ever be greater than 0xfe. >> > > >> > > As is, the best you can expect from that function is a long >> > > second argument for std::max(). Unless I'm completely missing >> > > something here (and I'll certainly budget for that possibility). >> > >> > You're completely missing the point. >> > >> > We provide an opportunity to the entire community to fix things >> > in the best way possible. >> > >> > And you squander it. >> > >> > The solution is to take all the subsystems and arguments to time_t, >> > throughout the entire program, and then do the best from there. >> > >> > Instead, you chose to prematurely downcast. >> >> Now, before you say this is impossible or ridiculous, let me say this. >> >> We did what I suggest to the entire base tree, before the kernel >> changes were commited. If we can do this to the entire base tree >> system -- something much much larger than the average package, then >> people working in the ports tree can at least try to not squander >> the opportunity. >> >> Please & thanks. > > > unsquandering ... (hopefully) > > The reason for the extra REVISION bump is for ajacoutot@'s > patch-src_Toolbar_cc addition (it missed the bump?). > > Minimally tested.
well? Also, would like to (again) bring attention to this patch: http://marc.info/?l=openbsd-ports&m=137339931231574&w=2 --patrick > Index: Makefile > =================================================================== > RCS file: /cvs/obsd/ports/x11/blackbox/Makefile,v > retrieving revision 1.51 > diff -u -p -u -p -r1.51 Makefile > --- Makefile 21 Mar 2013 08:48:55 -0000 1.51 > +++ Makefile 18 Aug 2013 07:14:22 -0000 > @@ -1,9 +1,9 @@ > -# $OpenBSD: Makefile,v 1.50 2013/03/11 11:46:08 espie Exp $ > +# $OpenBSD: Makefile,v 1.51 2013/03/21 08:48:55 ajacoutot Exp $ > > COMMENT= small & pretty window manager for 8 and more bits displays > > DISTNAME= blackbox-0.70.1 > -REVISION= 2 > +REVISION= 4 > CATEGORIES= x11 > > HOMEPAGE= http://blackboxwm.sourceforge.net/ > Index: patches/patch-lib_Resource_cc > =================================================================== > RCS file: patches/patch-lib_Resource_cc > diff -N patches/patch-lib_Resource_cc > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-lib_Resource_cc 18 Aug 2013 07:14:22 -0000 > @@ -0,0 +1,20 @@ > +$OpenBSD$ > + > +64bit time_t fix. > + > +--- lib/Resource.cc.orig Wed Apr 6 14:16:50 2005 > ++++ lib/Resource.cc Sat Aug 17 22:26:11 2013 > +@@ -207,6 +207,13 @@ void bt::Resource::write(const char* resource, > unsigne > + } > + > + > ++void bt::Resource::write(const char* resource, unsigned long long value) > { > ++ char tmp[64]; > ++ sprintf(tmp, "%llu", value); > ++ write(resource, tmp); > ++} > ++ > ++ > + void bt::Resource::write(const char* resource, bool value) > + { write(resource, boolAsString(value)); } > + > Index: patches/patch-lib_Resource_hh > =================================================================== > RCS file: patches/patch-lib_Resource_hh > diff -N patches/patch-lib_Resource_hh > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-lib_Resource_hh 18 Aug 2013 07:14:22 -0000 > @@ -0,0 +1,14 @@ > +$OpenBSD$ > + > +64bit time_t fix. > + > +--- lib/Resource.hh.orig Mon Jan 3 01:42:52 2005 > ++++ lib/Resource.hh Sat Aug 17 22:25:59 2013 > +@@ -78,6 +78,7 @@ namespace bt { > + void write(const char* resource, unsigned int value); > + void write(const char* resource, long value); > + void write(const char* resource, unsigned long value); > ++ void write(const char* resource, unsigned long long value); > + void write(const char* resource, bool value); > + void write(const char* resource, double value); > + > Index: patches/patch-lib_Timer_cc > =================================================================== > RCS file: patches/patch-lib_Timer_cc > diff -N patches/patch-lib_Timer_cc > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-lib_Timer_cc 18 Aug 2013 07:14:22 -0000 > @@ -0,0 +1,25 @@ > +$OpenBSD$ > + > +64bit time_t fix. > + > +--- lib/Timer.cc.orig Fri Mar 18 01:07:09 2005 > ++++ lib/Timer.cc Sat Aug 17 22:43:13 2013 > +@@ -28,8 +28,7 @@ > + > + > + bt::timeval::timeval(const ::timeval &t) > +- : tv_sec(t.tv_sec), tv_usec(t.tv_usec) > +-{ } > ++{ tv_sec = t.tv_sec; tv_usec = t.tv_usec; } > + > + > + bool bt::timeval::operator<(const timeval &tv) > +@@ -105,7 +104,7 @@ void bt::Timer::adjustStartTime(const timeval &offset) > + { _start += offset; } > + > + > +-void bt::Timer::setTimeout(long t) { > ++void bt::Timer::setTimeout(time_t t) { > + _timeout.tv_sec = t / 1000; > + _timeout.tv_usec = t % 1000; > + _timeout.tv_usec *= 1000; > Index: patches/patch-lib_Timer_hh > =================================================================== > RCS file: patches/patch-lib_Timer_hh > diff -N patches/patch-lib_Timer_hh > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-lib_Timer_hh 18 Aug 2013 07:14:22 -0000 > @@ -0,0 +1,36 @@ > +$OpenBSD$ > + > +64bit time_t fix. > + > +--- lib/Timer.hh.orig Fri Mar 18 01:07:09 2005 > ++++ lib/Timer.hh Sat Aug 17 22:43:03 2013 > +@@ -37,16 +37,12 @@ struct timeval; > + namespace bt { > + > + // use a wrapper class to avoid the header as well > +- struct timeval { > +- long tv_sec; > +- long tv_usec; > ++ struct timeval : public ::timeval { > + > + inline timeval(void) > +- : tv_sec(0l), tv_usec(0l) > +- { } > +- inline timeval(long s, long u) > +- : tv_sec(s), tv_usec(u) > +- { } > ++ { tv_sec = 0; tv_usec = 0; } > ++ inline timeval(time_t s, long u) > ++ { tv_sec = s; tv_usec = u; } > + > + bool operator<(const timeval &); > + timeval operator+(const timeval &); > +@@ -106,7 +102,7 @@ namespace bt { > + inline void recurring(bool b) > + { recur = b; } > + > +- void setTimeout(long t); > ++ void setTimeout(time_t t); > + void setTimeout(const timeval &t); > + > + void start(void); // manager acquires timer > Index: patches/patch-src_Toolbar_cc > =================================================================== > RCS file: /cvs/obsd/ports/x11/blackbox/patches/patch-src_Toolbar_cc,v > retrieving revision 1.1 > diff -u -p -u -p -r1.1 patch-src_Toolbar_cc > --- patches/patch-src_Toolbar_cc 16 Aug 2013 19:37:45 -0000 1.1 > +++ patches/patch-src_Toolbar_cc 18 Aug 2013 07:14:22 -0000 > @@ -1,18 +1,24 @@ > $OpenBSD$ > > -Fix build failure when time_t is long long. > +64bit time_t fix. > > ---- src/Toolbar.cc.orig Tue Apr 12 09:38:00 2005 > -+++ src/Toolbar.cc Fri Aug 16 21:33:33 2013 > -@@ -43,8 +43,10 @@ > - long nextTimeout(int resolution) > +--- src/Toolbar.cc.orig Tue Apr 12 00:38:00 2005 > ++++ src/Toolbar.cc Sat Aug 17 22:47:24 2013 > +@@ -40,12 +40,14 @@ > + #include <assert.h> > + > + > +-long nextTimeout(int resolution) > ++time_t nextTimeout(int resolution) > { > timeval now; > -+ int secs; > gettimeofday(&now, 0); > - return (std::max(1000l, ((((resolution - (now.tv_sec % resolution)) * > 1000l)) > -+ secs = now.tv_sec % resolution; > -+ return (std::max(1000l, ((((resolution - secs) * 1000l)) > - - (now.tv_usec / 1000l)))); > +- - (now.tv_usec / 1000l)))); > ++ time_t t; > ++ t = (resolution - (now.tv_sec % resolution)) * 1000l; > ++ t -= now.tv_usec / 1000l; > ++ return std::max(t, (time_t)1000); > } > + > >
