Junio C Hamano [off-list ref] writes:
You've taught people here and on the kernel list that the "date" can use
any non-digit-non-word as a word separator, and "git log --since 2.days"
is something you often do.
People who followed that advice would have gotten used to this already, e.g.
$ git reflog delete master@{07.04.2005.15:15:00.-0700}
should not be broken.
I think your first hunk needs to distinguish between "very-long-precision
posint" (in which case we ignore because it is likely to be nanoseconds
fraction) and others.
Perhaps like this.
-- >8 --
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sat, 16 Aug 2008 16:17:50 -0700
Subject: [PATCH] date parsing: do not mistake fractional nanosecond that follow HH:MM:SS
Some program output nanosecond fractional after the usual HH:MM:SS format.
If the fraction is large enough, it can be interpreted as the seconds
since epoch, and can overwrite the already parsed date/time.
We also make sure we use the seconds since epoch interpretation only when
we have not seen any other date/time data in the input yet.
Note that we cannot unconditionally drop anything that follows '.'; people
have been taught that we allow '.' as a word separator and have got used
to formats like "--since 2.days" and "2008.08.16.01:23:45.-0700" to work.
Noticed-by: Hermann Gausterer
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <redacted>
---
date.c | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/date.c b/date.c
index 35a5257..b2c5a8b 100644
--- a/date.c
+++ b/date.c
@@ -363,6 +363,11 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
tm->tm_hour = num;
tm->tm_min = num2;
tm->tm_sec = num3;
+ if (*end == '.') {
+ num = strspn(end+1, "0123456789");
+ if (9 <= num)
+ end += num + 1;
+ }
break;
}
return 0;@@ -402,6 +407,15 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
return end - date;
}
+/* Have we filled in any part of the time/date yet? */
+static inline int nodate(struct tm *tm)
+{
+ return tm->tm_year < 0 &&
+ tm->tm_mon < 0 &&
+ tm->tm_mday < 0 &&
+ !(tm->tm_hour | tm->tm_min | tm->tm_sec);
+}
+
/*
* We've seen a digit. Time? Year? Date?
*/@@ -418,7 +432,7 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
* more than 8 digits. This is because we don't want to rule out
* numbers like 20070606 as a YYYYMMDD date.
*/
- if (num >= 100000000) {
+ if (num >= 100000000 && nodate(tm)) {
time_t time = num;
if (gmtime_r(&time, tm)) {
*tm_gmt = 1;--
1.6.0.rc3.17.gc14c8
On Sat, 16 Aug 2008, Junio C Hamano wrote:
Perhaps like this.
So see in the previous email why I don't think "ignore nanoseconds" is
really any better than "igore all fractions".
That said:
quoted hunk
@@ -363,6 +363,11 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
tm->tm_hour = num;
tm->tm_min = num2;
tm->tm_sec = num3;
+ if (*end == '.') {
+ num = strspn(end+1, "0123456789");
+ if (9 <= num)
+ end += num + 1;
Apart from the "compare with 9", your patch is _much_ better than mine.
Using "strtoul()" was a horrible horrible thing to do, since it will match
not just '+' and '-' but also spaces etc.
So my patch was definitely crap, and yours is better, but I don't much
like that expectations of 9+ digits. After all, if we only worry about 9+
digits of a big number, then the "nodate()" logic already takes care of
much of it.
So here's a much better version, I think.
The rules are:
- valid days of month/mday are always single or double digits.
- valid years are either two or four digits
No, we don't support the year 600 _anyway_, since our encoding is based
on the UNIX epoch, and the day we worry about the year 10,000 is far
away and we can raise the limit to five digits when we get closer.
- Other numbers (eg "600 days ago") can have any number of digits, but
they cannot start with a zero. Again, the only exception is for
two-digit numbers, since that is fairly common for dates ("Dec 01" is
not unheard of)
So that means that any milli- or micro-second would be thrown out just
because the number of digits shows that it cannot be an interesting date.
That would make the patch look something like this...
[ Those four deleted lines I removed just because the cases had already
been handled, eg the ">1900" case was already handled when we checked
for a four-digit year, and the >70 case was handled when we checked for
exactly two digits ]
Hmm?
Linus
---
date.c | 26 ++++++++++++++++++++------
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/date.c b/date.c
index 35a5257..950b88f 100644
--- a/date.c
+++ b/date.c
@@ -402,6 +402,15 @@ static int match_multi_number(unsigned long num, char c, const char *date, char
return end - date;
}
+/* Have we filled in any part of the time/date yet? */
+static inline int nodate(struct tm *tm)
+{
+ return tm->tm_year < 0 &&
+ tm->tm_mon < 0 &&
+ tm->tm_mday < 0 &&
+ !(tm->tm_hour | tm->tm_min | tm->tm_sec);
+}
+
/*
* We've seen a digit. Time? Year? Date?
*/@@ -418,7 +427,7 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
* more than 8 digits. This is because we don't want to rule out
* numbers like 20070606 as a YYYYMMDD date.
*/
- if (num >= 100000000) {
+ if (num >= 100000000 && nodate(tm)) {
time_t time = num;
if (gmtime_r(&time, tm)) {
*tm_gmt = 1;@@ -463,6 +472,13 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
}
/*
+ * Ignore lots of numerals. We took care of 4-digit years above.
+ * Days or months must be one or two digits.
+ */
+ if (n > 2)
+ return n;
+
+ /*
* NOTE! We will give precedence to day-of-month over month or
* year numbers in the 1-12 range. So 05 is always "mday 5",
* unless we already have a mday..
@@ -488,10 +504,6 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
if (num > 0 && num < 32) {
tm->tm_mday = num;
- } else if (num > 1900) {
- tm->tm_year = num - 1900;
- } else if (num > 70) {
- tm->tm_year = num;
} else if (num > 0 && num < 13) {
tm->tm_mon = num-1;
}@@ -823,7 +835,9 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
}
}
- *num = number;
+ /* Accept zero-padding only for small numbers ("Dec 02", never "Dec 0002") */
+ if (date[0] != '0' || end - date <= 2)
+ *num = number;
return end;
}
On Sat, 16 Aug 2008, Linus Torvalds wrote:
The rules are:
- valid days of month/mday are always single or double digits.
- valid years are either two or four digits
No, we don't support the year 600 _anyway_, since our encoding is based
on the UNIX epoch, and the day we worry about the year 10,000 is far
away and we can raise the limit to five digits when we get closer.
- Other numbers (eg "600 days ago") can have any number of digits, but
they cannot start with a zero. Again, the only exception is for
two-digit numbers, since that is fairly common for dates ("Dec 01" is
not unheard of)
So that means that any milli- or micro-second would be thrown out just
because the number of digits shows that it cannot be an interesting date.
I should explain that nonsensical statement a bit.
A milli- or micro-second can obviously be a perfectly fine number
according to the rules above, as long as it doesn't start with a '0'. So
if we have
12:34:56.123
then that '123' gets parsed as a number, and we remember it. But because
it's bigger than 31, we'll never use it as such _unless_ there is
something after it to trigger that use.
So you can say "12:34:56.123.days.ago", and because of the "days", that
123 will actually be meaninful now.
But the problem with "12.34.56.001" was that we used to remember the "001"
as a number, and because we could see no other use for it we then assumed
that it meant the day of the month.
Of course, we *should* do that only if we have seen a month-name too, but
we don't currently track that, so.. Adding that as a further sanity test
would be good, but it would require us to have some extra "flags" field.
Maybe we should have a
#define SEEN_MONTH 1
#define SEEN_YEAR 2
#define SEEN_DAY 4
#define SEEN_TIME 8
...
struct extended_tm {
unsigned long seen;
unsigned long number;
struct tm tm;
}
and pass *that* around instead of passing "struct tm *" and "unsigned long
*num" around. That would be good. Then we could do
if (tm->number && tm->number < 32 &&
(tm->seen & SEEN_MONTH) && !(tm->seen & SEEN_DAY))
tm->tm.tm_mday = tm->number;
there instead, which would protect us from other numbers just being seen
as days instead.
Anybody? I'm not going to bother.
Linus