Am 22.09.19 um 21:53 schrieb SZEDER Gábor:
> On Sun, Sep 22, 2019 at 07:57:36PM +0100, Phillip Wood wrote:
>> On 22/09/2019 19:01, SZEDER Gábor wrote:
>>> +/*
>>> + * One day. See the 'name a rev close to epoch' test in t6120 when
>>> + * changing this value
>>> + */
>>> +#define CUTOFF_DATE_SLOP 86400
>>> typedef struct rev_name {
>>> const char *tip_name;
>>> @@ -481,8 +485,13 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
>>> add_object_array(object, *argv, &revs);
>>> }
>>> - if (cutoff)
>>> - cutoff = cutoff - CUTOFF_DATE_SLOP;
>>> + if (cutoff) {
>>> + /* check for undeflow */
>>> + if (cutoff - CUTOFF_DATE_SLOP < cutoff)
>>
>> Nice catch but wouldn't this be clearer as
>> if (cutoff > CUTOFF_DATE_SLOP) ?
>
> It would only be clearer now, with an unsigned 'timestamp_t'. I
> tried to future-proof for a signed 'timestamp_t' and a cutoff date
> before the UNIX epoch.
Huh? For signed cutoff and positive CUTOFF_DATE_SLOP,
cutoff - CUTOFF_DATE_SLOP < cutoff is ALWAYS true. Signed interger
underflow is undefined behavior and signed integer arithmetic does not
wrap around!
IOW, the new condition makes only sense today, because cutoff is an
unsigned type, but breaks down should we switch to a signed type.
You need this on top:
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a4d8d312ab..2d83c2b172 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -487,10 +487,10 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
if (cutoff) {
/* check for undeflow */
- if (cutoff - CUTOFF_DATE_SLOP < cutoff)
+ if (cutoff > TIME_MIN + CUTOFF_DATE_SLOP)
cutoff = cutoff - CUTOFF_DATE_SLOP;
else
- cutoff = 0;
+ cutoff = TIME_MIN;
}
for_each_ref(name_ref, &data);
diff --git a/git-compat-util.h b/git-compat-util.h
index c68c61d07c..1bdc21a069 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -344,6 +344,7 @@ typedef uintmax_t timestamp_t;
#define PRItime PRIuMAX
#define parse_timestamp strtoumax
#define TIME_MAX UINTMAX_MAX
+#define TIME_MIN 0
#ifndef PATH_SEP
#define PATH_SEP ':'
