On Tue, Dec 12, 2017 at 12:55:27PM +0200, Pekka Paalanen wrote: > On Tue, 12 Dec 2017 12:36:39 +0200 > Alexandros Frantzis <[email protected]> wrote: > > > On Tue, Dec 12, 2017 at 11:50:49AM +0200, Pekka Paalanen wrote: > > > On Mon, 4 Dec 2017 15:34:01 +0200 > > > Alexandros Frantzis <[email protected]> wrote: > > > > > > > Add a helper function to normalize struct timespec values so that the > > > > nanoseconds part is less than 1 second and has the same sign as the > > > > seconds part (if the seconds part is not 0). > > > > > > > > Normalization is required to ensure we can safely convert timespec > > > > values to wayland protocol data, i.e, to tv_sec_hi, tv_sec_lo, > > > > tv_sec_nsec triplets, and will be used in upcoming commits. > > > > > > > > Signed-off-by: Alexandros Frantzis <[email protected]> > > > > --- > > > > shared/timespec-util.h | 28 ++++++++++++++++++++++ > > > > tests/timespec-test.c | 65 > > > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > 2 files changed, 93 insertions(+) > > > > > > > > diff --git a/shared/timespec-util.h b/shared/timespec-util.h > > > > index f9736c27..a10edf5b 100644 > > > > --- a/shared/timespec-util.h > > > > +++ b/shared/timespec-util.h > > > > @@ -33,6 +33,34 @@ > > > > > > > > #define NSEC_PER_SEC 1000000000 > > > > > > > > +/* Normalize a timespec > > > > + * > > > > + * \param r[out] normalized timespec > > > > + * \param a[in] timespec to normalize > > > > + * > > > > + * Normalize a timespec so that tv_nsec is less than 1 second > > > > + * and has the same sign as tv_sec (if tv_sec is non-zero). > > > > > > Hi, > > > > Hi Pekka, > > > > thanks for the review. > > > > > why does it need to have the same sign? > > > E.g. timespec_sub() ensures nsec is non-negative, as do the add > > > functions. > > > > The goal was to make timespec_normalize more generally useful and > > independent of any current behavior. I didn't want to assume that the > > provided timespec would be produced necessarily by using the > > aforementioned functions that already provide the same-sign guarantee. > > They don't provide the same-sign guarantee. They ensure nsec is > non-negative, regardless of the sign of sec.
Ah, I misunderstood. > The question here is, what is the normalized form? > > We have not had use for negative time values in the protocol, and the > new proposal does not either, so protocol specs do not offer any > precendent. Computationally they could appear in programs, so the > definition of "normalized" for these helper functions will be purely > libweston-internal. > > I'd go with non-negative instead of same-sign, because the existing > code is already like that. I'd like to hear about any better > justification one way or another, since I have little else. I believe > non-negative lead to simpler code. The main justification for same sign is a more intuitive representation for humans. That is, it's more natural to think of -1.5 as -1 + -0.5, rather than -2 + 0.5. However, that's not a problem from the computer's point of view and I had missed the pre-existing non-negative nsec guarantee in other functions. Also, since in the normalized form nsec < 1 sec, checking the sign of tv_sec is enough to determine the sign of the timespec which is a nice property to maintain. So, bottom line, I am convinced. I will change timespec_normalize to provide the non-negative nsec guarantee. Thanks, Alexandros _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
