diff mbox series

[Outreachy,1/2] date: refactor 64 bit prereq code into reusable functions

Message ID 20240205162506.1835-1-ach.lumap@gmail.com (mailing list archive)
State New, archived
Headers show
Series [Outreachy,1/2] date: refactor 64 bit prereq code into reusable functions | expand

Commit Message

Achu Luma Feb. 5, 2024, 4:25 p.m. UTC
In a following commit we are going to port code from
"t/helper/test-date.c" and "t/t0006-date.sh" to a new
"t/unit-tests/t-date.c" file using the recently added unit test
framework.

We cannot fully port all the code from "t/helper/test-date.c" though, as
the test-tool date helper is still used by a number of "t/*.sh" tests.
The TIME_IS_64BIT and TIME_T_IS_64BIT prereqs are especially used by
"t5000-tar-tree.sh", "t5318-commit-graph.sh" and
"t5328-commit-graph-64bit-time.sh" while checking those prereqs will be
required in the new "t/unit-tests/t-date.c" file too.

To avoid duplicating in both "t/helper/test-date.c" and
"t/unit-tests/t-date.c" the small amount of code checking these prereqs,
let's move it into inline functions in "date.h".

The names of these new inline functions contain "TIME_IS_64BIT" or
"TIME_T_IS_64BIT" as it will simplify the macros we will use when
we will port code to "t/unit-tests/t-date.c" in a following commit.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Achu Luma <ach.lumap@gmail.com>
---
 date.h               | 6 ++++++
 t/helper/test-date.c | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

--
2.43.0.windows.1

Comments

Randall S. Becker Feb. 5, 2024, 5:34 p.m. UTC | #1
On Monday, February 5, 2024 11:25 AM, Achu Luma wrote:
>In a following commit we are going to port code from "t/helper/test-date.c"
and
>"t/t0006-date.sh" to a new "t/unit-tests/t-date.c" file using the recently
added unit
>test framework.
>
>We cannot fully port all the code from "t/helper/test-date.c" though, as
the test-
>tool date helper is still used by a number of "t/*.sh" tests.
>The TIME_IS_64BIT and TIME_T_IS_64BIT prereqs are especially used by
"t5000-
>tar-tree.sh", "t5318-commit-graph.sh" and
"t5328-commit-graph-64bit-time.sh"
>while checking those prereqs will be required in the new
"t/unit-tests/t-date.c" file
>too.
>
>To avoid duplicating in both "t/helper/test-date.c" and
"t/unit-tests/t-date.c" the
>small amount of code checking these prereqs, let's move it into inline
functions in
>"date.h".
>
>The names of these new inline functions contain "TIME_IS_64BIT" or
>"TIME_T_IS_64BIT" as it will simplify the macros we will use when we will
port code
>to "t/unit-tests/t-date.c" in a following commit.
>
>Mentored-by: Christian Couder <chriscool@tuxfamily.org>
>Signed-off-by: Achu Luma <ach.lumap@gmail.com>
>---
> date.h               | 6 ++++++
> t/helper/test-date.c | 4 ++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/date.h b/date.h
>index 6136212a19..fb70490a51 100644
>--- a/date.h
>+++ b/date.h
>@@ -70,4 +70,10 @@ void datestamp(struct strbuf *out);  timestamp_t
>approxidate_careful(const char *, int *);  int date_overflows(timestamp_t
date);
>time_t tm_to_time_t(const struct tm *tm);
>+static inline int check_prereq_TIME_IS_64BIT(void) {
>+	return sizeof(timestamp_t) == 8;
>+}
>+static inline int check_prereq_TIME_T_IS_64BIT(void) {
>+	return sizeof(time_t) == 8;
>+}
> #endif
>diff --git a/t/helper/test-date.c b/t/helper/test-date.c index
>0683d46574..be0b8679c3 100644
>--- a/t/helper/test-date.c
>+++ b/t/helper/test-date.c
>@@ -126,9 +126,9 @@ int cmd__date(int argc UNUSED, const char **argv)
> 	else if (!strcmp(*argv, "getnanos"))
> 		getnanos(argv+1);
> 	else if (!strcmp(*argv, "is64bit"))
>-		return sizeof(timestamp_t) == 8 ? 0 : 1;
>+		return !check_prereq_TIME_IS_64BIT();
> 	else if (!strcmp(*argv, "time_t-is64bit"))
>-		return sizeof(time_t) == 8 ? 0 : 1;
>+		return !check_prereq_TIME_T_IS_64BIT();
> 	else
> 		usage(usage_msg);
> 	return 0;
>--
>2.43.0.windows.1

I would suggest that you also take into account whether time_t is signed or
not (more difficult perhaps). Some platforms use signed time_t to allow
representation of dates prior to 1970-01-01, while others make this signed.
Some other platforms (S/390 for example) have retained time_t as 32-bits but
have a time64_t for 64 bits. It might be useful to account for this.
--Randall
Christian Couder Feb. 6, 2024, 8:39 a.m. UTC | #2
On Mon, Feb 5, 2024 at 6:34 PM <rsbecker@nexbridge.com> wrote:
> On Monday, February 5, 2024 11:25 AM, Achu Luma wrote:

> I would suggest that you also take into account whether time_t is signed or
> not (more difficult perhaps). Some platforms use signed time_t to allow
> representation of dates prior to 1970-01-01, while others make this signed.
> Some other platforms (S/390 for example) have retained time_t as 32-bits but
> have a time64_t for 64 bits. It might be useful to account for this.

The goal of this small series is just to port some existing tests to
the new unit test framework. I think it's a different topic to improve
the existing tests to take into account whether time_t is signed or
not. But thanks for the info.
diff mbox series

Patch

diff --git a/date.h b/date.h
index 6136212a19..fb70490a51 100644
--- a/date.h
+++ b/date.h
@@ -70,4 +70,10 @@  void datestamp(struct strbuf *out);
 timestamp_t approxidate_careful(const char *, int *);
 int date_overflows(timestamp_t date);
 time_t tm_to_time_t(const struct tm *tm);
+static inline int check_prereq_TIME_IS_64BIT(void) {
+	return sizeof(timestamp_t) == 8;
+}
+static inline int check_prereq_TIME_T_IS_64BIT(void) {
+	return sizeof(time_t) == 8;
+}
 #endif
diff --git a/t/helper/test-date.c b/t/helper/test-date.c
index 0683d46574..be0b8679c3 100644
--- a/t/helper/test-date.c
+++ b/t/helper/test-date.c
@@ -126,9 +126,9 @@  int cmd__date(int argc UNUSED, const char **argv)
 	else if (!strcmp(*argv, "getnanos"))
 		getnanos(argv+1);
 	else if (!strcmp(*argv, "is64bit"))
-		return sizeof(timestamp_t) == 8 ? 0 : 1;
+		return !check_prereq_TIME_IS_64BIT();
 	else if (!strcmp(*argv, "time_t-is64bit"))
-		return sizeof(time_t) == 8 ? 0 : 1;
+		return !check_prereq_TIME_T_IS_64BIT();
 	else
 		usage(usage_msg);
 	return 0;