Deciding whether time values should be signed or unsigned

[Peter McCurdy]

On Mon, Aug 25, 2008 at 2:50 PM, Havoc Pennington <hp at pobox.com> wrote:
> Hi,
>
> On Mon, Aug 25, 2008 at 2:00 PM, Thiago Macieira <thiago at kde.org> wrote:
>> You should use time_t, struct timeval, etc. internally, everywhere when
>> dealing with the system calls. Don't try to invent your own time value.
>>
>> I also don't see the need to have sec/usec pairs everywhere. A struct would
>> probably be better, and we have "timeval" for that. I understand the aversion
>> of using any system headers in files that aren't "sysdep", so we could define
>> our own structure.
>
> Cleaning up the internal API would not be bad, I think it's a separate
> patch from just fixing the warnings though. (Meaning both, ideally it
> is a separate patch for purposes of git history and patch review, and
> that it's OK to go ahead and fix the warnings without a larger
> refactoring)
>
> If cleaning up the internal API, I would be reluctant to use struct
> timeval and time_t outside sysdeps as you say.

You'd also want some sensible functions to add, subtract, compare,
etc. these time values.  The lack of these is part of what makes the
current code bulkier than it needs to be.

If you're going to make your own time struct anyway, you may want to
consider just using a 64-bit count of microseconds, or something
similar.  You'd get the same precision as you currently have, have a
useful range (+/- 300,000 years), not have to deal with creating
structs and passing them around, and have an easier go with the time
arithmetic.  The drawback would be having to do some conversion
whenever you interact with the outside world.  Anyway, not really
relevant right at the moment, just something to consider.

>> I understand that the last two points don't apply to the patches you have, but
>> they come as indications that signed is preferred.
>
> Right
>

Thiago's points make a lot of sense.  So that makes it roughly 3-0 in
favour of the signed patch, with justification and everything.

Does anyone have any comments about the actual patch itself?

Thanks.

Peter.