[PATCH] script: don't assume time_t is compatible with long

[PATCH] script: don't assume time_t is compatible with long

[Andreas Schwab]

Signed-off-by: Andreas Schwab <schwab@suse.de>
---
 term-utils/script.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/term-utils/script.c b/term-utils/script.c
index eb4ddc3..ad252a3 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -141,7 +141,7 @@ static void script_init_debug(void)
 static inline time_t script_time(time_t *t)
 {
 	const char *str = getenv("SCRIPT_TEST_SECOND_SINCE_EPOCH");
-	time_t sec;
+	long sec;
 
 	if (str && sscanf(str, "%ld", &sec) == 1)
 		return sec;
-- 
2.6.1


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] script: don't assume time_t is compatible with long

[Isaac Dunham]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

On Thu, Oct 15, 2015 at 03:06:04PM +0200, Andreas Schwab wrote:
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
>  term-utils/script.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/term-utils/script.c b/term-utils/script.c
> index eb4ddc3..ad252a3 100644
> --- a/term-utils/script.c
> +++ b/term-utils/script.c
> @@ -141,7 +141,7 @@ static void script_init_debug(void)
>  static inline time_t script_time(time_t *t)
>  {
>  	const char *str = getenv("SCRIPT_TEST_SECOND_SINCE_EPOCH");
> -	time_t sec;
> +	long sec;
>  
>  	if (str && sscanf(str, "%ld", &sec) == 1)
>  		return sec;

I don't think this does what the commit message says.
Rather, it moves the assumption.
If you're trying to actually *fix* it so it works with 64-bit time_t on
x86 (some kernel developers have discussed a path forwards on that, and
OpenBSD has already implemented it), this will not do the job.

And I note that the old code here is already technically wrong, since this
is supposed to a replacement for time(). It should have included something
equivalent to:
  if (t)
    *t = (time_t)sec;


I'm guessing that the attached patch would be the most corrrect approach;
any comments?

Thanks,
Isaac Dunham

[-- Attachment #2: 0001-script-don-t-assume-that-time_t-is-compatible-with-l.patch --]
[-- Type: text/plain, Size: 1062 bytes --]

>From 4fc3751060ab5d4fb84aa814520c7ca1afe32a28 Mon Sep 17 00:00:00 2001
From: Isaac Dunham <ibid.ag@gmail.com>
Date: Thu, 15 Oct 2015 18:03:28 -0700
Subject: [PATCH] script: don't assume that time_t is compatible with long

time_t may change to 64-bit on 32-bit Linux kernels at some point;
at that point, it may be desireable to test for issues with dates
past 2038.
---
 term-utils/script.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/term-utils/script.c b/term-utils/script.c
index eb4ddc3..f0e997e 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -141,11 +141,13 @@ static void script_init_debug(void)
 static inline time_t script_time(time_t *t)
 {
 	const char *str = getenv("SCRIPT_TEST_SECOND_SINCE_EPOCH");
-	time_t sec;
+	int64_t sec;
 
-	if (str && sscanf(str, "%ld", &sec) == 1)
-		return sec;
-	return time(t);
+	if (!str || sscanf(str, "%lld", &sec) != 1)
+		return time(t);
+	if (t)
+		*t = (time_t)sec;
+	return (time_t)sec;
 }
 #else	/* !TEST_SCRIPT */
 # define script_time(x) time(x)
-- 
2.6.1

Re: [PATCH] script: don't assume time_t is compatible with long

[Karel Zak]

On Thu, Oct 15, 2015 at 06:12:59PM -0700, Isaac Dunham wrote:
> I'm guessing that the attached patch would be the most corrrect approach;
> any comments?

 Applied,

> +	if (!str || sscanf(str, "%lld", &sec) != 1)

 Try compile with -Wformat, compiler does not like
 %ll for int64_t types :-)
 
 We usually use %jd and %ju for 64-bit numbers, but real pedantic
 solution is to use SCN macros ("%"SCNi64 in this case) from
 inttypes.h.

 Hmm.. is there any system where intmax_t is already 128-bits? I know
 that gcc already supports __int128_t on some archs, but it's not
 treated as extended integer types. So, I guess we're relative safe
 when we assume that intmax_t is 64-bits :-)

 http://stackoverflow.com/questions/29927562/what-abi-if-any-restricts-the-size-of-uintmax-t

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com