[PATCH weston 1/7] timespec: Add timespec_add_nsec helper

[Daniel Stone]

Hi,

On 14 February 2017 at 15:02, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Tue, 14 Feb 2017 14:21:17 +0000
> Daniel Stone <daniel at fooishbar.org> wrote:
>> On 14 February 2017 at 14:09, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> > On Tue, 14 Feb 2017 13:18:01 +0000
>> > Daniel Stone <daniels at collabora.com> wrote:
>> >> +static inline void
>> >> +timespec_add_nsec(struct timespec *r, const struct timespec *a, int64_t b)
>> >> +{
>> >> +     r->tv_sec = a->tv_sec + (b / NSEC_PER_SEC);
>> >> +     r->tv_nsec = a->tv_nsec + (b % NSEC_PER_SEC);
>> >> +
>> >> +     if (r->tv_nsec >= NSEC_PER_SEC) {
>> >> +             r->tv_sec++;
>> >> +             r->tv_nsec -= NSEC_PER_SEC;
>> >> +     } else if (r->tv_nsec <= -NSEC_PER_SEC) {
>> >
>> > IIRC tv_nsec cannot be negative, so it should be:
>> >         r->tv_nsec < 0
>>
>> This is working on r->tv_nsec, i.e. (a->tv_nsec + signed input value).
>> I did this to deliberately allow timespec_add_nsec(&r, &a, -12345).
>
> Indeed r->tv_nsec, and you need to guarantee to not return a timespec
> with an invalid tv_nsec. It has no relation to allowing negative b
> which is fine.

Oh, I see what you mean. I think this was related to confusion on what
a normalised timespec looked like with negative values.

>> >> +     a.tv_sec = 0;
>> >> +     a.tv_nsec = 1;
>> >> +     timespec_add_nsec(&r, &a, -2);
>> >> +     ZUC_ASSERT_EQ(r.tv_sec, 0);
>> >> +     ZUC_ASSERT_EQ(r.tv_nsec, -1);
>> >
>> > Incorrect, tv_nsec cannot be negative.
>>
>> So this would have to be { .tv_sec = -1, .tv_nsec = NSEC_PER_SEC - 1
>> }? I was somewhat confused as to the relationship of the two.
>
> Yes, nsec is always [0, 999999999] in normalized form.
>
> I was also confused about how to interpret a timespec when I started
> using it, but in the end, it is just tv_sec + tv_nsec / 1e9. I think
> comparison functions might become annoying if tv_nsec was allowed to be
> negative. Sticking to the normalized form makes things easier, IMO. At
> least it pays off to stick to just one normalized form (see
> presentation-time). :-)
>
> The real confusion comes if one tries to think of it as integer
> concatenated with fractional part ("%d.%09d"). It works if tv_sec and
> tv_nsec are both positive (and normalized), but fails immediately when
> either becomes negative.

That's fine; now I know what the normalised form is supposed to be, I
can respin to actually conform to that. ;)

Cheers,
Daniel