[systemd-devel] [PATCH] time-util: accept epoch timetamps prefixed with @

[Dave Reisner]

On Sun, Mar 23, 2014 at 10:04:00PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Sun, Mar 23, 2014 at 02:27:04PM -0400, Dave Reisner wrote:
> > On Sun, Mar 23, 2014 at 06:30:02PM +0100, Tollef Fog Heen wrote:
> > > ]] Dave Reisner 
> > > 
> > > > On Sun, Mar 23, 2014 at 05:27:08PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> > > > > On Sun, Mar 23, 2014 at 11:06:47AM -0400, Dave Reisner wrote:
> > > > > > Also adds a few tests for the absolute cases of parse_timestamp.
> > > > > Yeah, that looks useful.
> > > > > 
> > > > > You don't test negative values. Maybe you could an example with a negative
> > > > > value to the documentation and tests?
> > > > 
> > > > Negative epoch values? What would this represent?
> > > 
> > > Time before the epoch?
> > > 
> > > $ LANG=C date -d @-10000
> > > Wed Dec 31 22:13:20 CET 1969
> > > 
> > > So date(1) already understands this.
> > 
> > Uggh, do we really need to document this? I don't see why/where it would
> > actually be useful.
> Well, you don't need to document this explictly, i.e. there's no need to
> talk about negative values, since they're not really useful, but I think
> it should be mentioned that the '@' is followed by a signed integer.

Ah, I guess you're pointing me at systemd.time(7).

> > FWIW, parse_timestamp sort of already handles
> > negative values with the exception of anything that results in a
> > timestamp of -1 -- mktime can't distinguish between a real timestamp of
> > -1 and an invalid struct tm which it can't convert.
> > 
> >   $ journalctl --since='1969-12-31 18:59:59'
> >   Failed to parse timestamp: 1969-12-31 18:59:59
> > 
> > But then there's also some inconsistent behavior which parse_timestamp
> > already exposes when dealing with absolutes:
> > 
> >   $ journalctl --since='1969-12-31 18:59:58'
> >   -- Logs begin at Fri 2013-11-15 18:11:44 EST, end at Sun 2014-03-23 14:01:01 EDT. --
> >   <EOF>
> > 
> >   $ journalctl --since='-100 years'
> >   -- Logs begin at Fri 2013-11-15 18:11:44 EST, end at Sun 2014-03-23 14:01:01 EDT. --
> >   <logs follow>
> > 
> > I don't think this is going to work out so well when the return type
> > involed (usec_t) is unsigned...
> Are you sure? In your patch you use safe_atoi (not safe_atou). And on my
> machine, time_t is defined as __time_t, which is __TIME_T_TYPE, __SYSCALL_SLONG_TYPE,
> which looks signed to me. Actually, it seems that this safe_atoi should
> be replaced by assert_cc(sizeof(time_t)==sizeof(long)) and safe_atoli.

This isn't relevant to the problem I'm demonstrating. I agree that
time_t is a signed type. The trouble is with the outvalue from
parse_timestamps which is usec_t -- a systemd typedef which is uint64_t.

> Zbyszek
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel