* [PATCH 1/2] index-pack: rename struct thread_local
2024-11-17 1:31 ` [PATCH 0/2] C23 compatibility brian m. carlson
@ 2024-11-17 1:31 ` brian m. carlson
2024-11-17 1:31 ` [PATCH 2/2] reflog: rename unreachable brian m. carlson
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: brian m. carlson @ 2024-11-17 1:31 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Sam James
"thread_local" is a keyword in C23. To make sure that our code compiles
on a wide variety of C versions, rename struct thread_local to "struct
thread_local_data" to avoid a conflict.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste•net>
---
builtin/index-pack.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 9d23b41b3a..a6942233e5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -94,7 +94,7 @@ static LIST_HEAD(done_head);
static size_t base_cache_used;
static size_t base_cache_limit;
-struct thread_local {
+struct thread_local_data {
pthread_t thread;
int pack_fd;
};
@@ -117,7 +117,7 @@ static struct object_entry *objects;
static struct object_stat *obj_stat;
static struct ofs_delta_entry *ofs_deltas;
static struct ref_delta_entry *ref_deltas;
-static struct thread_local nothread_data;
+static struct thread_local_data nothread_data;
static int nr_objects;
static int nr_ofs_deltas;
static int nr_ref_deltas;
@@ -148,7 +148,7 @@ static uint32_t input_crc32;
static int input_fd, output_fd;
static const char *curr_pack;
-static struct thread_local *thread_data;
+static struct thread_local_data *thread_data;
static int nr_dispatched;
static int threads_active;
@@ -390,7 +390,7 @@ static NORETURN void bad_object(off_t offset, const char *format, ...)
(uintmax_t)offset, buf);
}
-static inline struct thread_local *get_thread_data(void)
+static inline struct thread_local_data *get_thread_data(void)
{
if (HAVE_THREADS) {
if (threads_active)
@@ -401,7 +401,7 @@ static inline struct thread_local *get_thread_data(void)
return ¬hread_data;
}
-static void set_thread_data(struct thread_local *data)
+static void set_thread_data(struct thread_local_data *data)
{
if (threads_active)
pthread_setspecific(key, data);
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/2] reflog: rename unreachable
2024-11-17 1:31 ` [PATCH 0/2] C23 compatibility brian m. carlson
2024-11-17 1:31 ` [PATCH 1/2] index-pack: rename struct thread_local brian m. carlson
@ 2024-11-17 1:31 ` brian m. carlson
2024-11-17 2:43 ` [PATCH 0/2] C23 compatibility Sam James
2024-11-18 7:45 ` Patrick Steinhardt
3 siblings, 0 replies; 8+ messages in thread
From: brian m. carlson @ 2024-11-17 1:31 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Sam James
In C23, "unreachable" is a macro that invokes undefined behavior if it
is invoked. To make sure that our code compiles on a variety of C
versions, rename unreachable to "is_unreachable".
Signed-off-by: brian m. carlson <sandals@crustytoothpaste•net>
---
reflog.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/reflog.c b/reflog.c
index 875ac1aa66..aeab78c9b7 100644
--- a/reflog.c
+++ b/reflog.c
@@ -210,7 +210,7 @@ static void mark_reachable(struct expire_reflog_policy_cb *cb)
cb->mark_list = leftover;
}
-static int unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit, struct object_id *oid)
+static int is_unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit, struct object_id *oid)
{
/*
* We may or may not have the commit yet - if not, look it
@@ -265,7 +265,7 @@ int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
return 1;
case UE_NORMAL:
case UE_HEAD:
- if (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))
+ if (is_unreachable(cb, old_commit, ooid) || is_unreachable(cb, new_commit, noid))
return 1;
break;
}
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 0/2] C23 compatibility
2024-11-17 1:31 ` [PATCH 0/2] C23 compatibility brian m. carlson
2024-11-17 1:31 ` [PATCH 1/2] index-pack: rename struct thread_local brian m. carlson
2024-11-17 1:31 ` [PATCH 2/2] reflog: rename unreachable brian m. carlson
@ 2024-11-17 2:43 ` Sam James
2024-11-18 7:45 ` Patrick Steinhardt
3 siblings, 0 replies; 8+ messages in thread
From: Sam James @ 2024-11-17 2:43 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano
"brian m. carlson" <sandals@crustytoothpaste•net> writes:
> Most of our code works fine in C23, but there are some new additions to
> the standard that conflict with either our struct tags or functions.
> With this series, the code compiles and passes the testsuite with
> -std=c23 on GCC 14.2.0.
>
> brian m. carlson (2):
> index-pack: rename struct thread_local
> reflog: rename unreachable
>
> builtin/index-pack.c | 10 +++++-----
> reflog.c | 4 ++--
> 2 files changed, 7 insertions(+), 7 deletions(-)
Thank you for the fast response! It both works and the fixes look
correct.
For the series:
Tested-by: Sam James <sam@gentoo•org>
Reviewed-by: Sam James <sam@gentoo•org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] C23 compatibility
2024-11-17 1:31 ` [PATCH 0/2] C23 compatibility brian m. carlson
` (2 preceding siblings ...)
2024-11-17 2:43 ` [PATCH 0/2] C23 compatibility Sam James
@ 2024-11-18 7:45 ` Patrick Steinhardt
2024-11-18 9:58 ` Jeff King
2024-11-18 22:11 ` brian m. carlson
3 siblings, 2 replies; 8+ messages in thread
From: Patrick Steinhardt @ 2024-11-18 7:45 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano, Sam James
On Sun, Nov 17, 2024 at 01:31:47AM +0000, brian m. carlson wrote:
> Most of our code works fine in C23, but there are some new additions to
> the standard that conflict with either our struct tags or functions.
> With this series, the code compiles and passes the testsuite with
> -std=c23 on GCC 14.2.0.
>
> brian m. carlson (2):
> index-pack: rename struct thread_local
> reflog: rename unreachable
>
> builtin/index-pack.c | 10 +++++-----
> reflog.c | 4 ++--
> 2 files changed, 7 insertions(+), 7 deletions(-)
Both of the patches look obviously good to me. I was a bit surprised
that this is required in the first place as I thought we were passing
`-std=gnu99` to our compilers, but that is not the case with our current
Makefile. So I must have been misremembering.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 0/2] C23 compatibility
2024-11-18 7:45 ` Patrick Steinhardt
@ 2024-11-18 9:58 ` Jeff King
2024-11-18 22:11 ` brian m. carlson
1 sibling, 0 replies; 8+ messages in thread
From: Jeff King @ 2024-11-18 9:58 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: brian m. carlson, git, Junio C Hamano, Sam James
On Mon, Nov 18, 2024 at 08:45:03AM +0100, Patrick Steinhardt wrote:
> Both of the patches look obviously good to me. I was a bit surprised
> that this is required in the first place as I thought we were passing
> `-std=gnu99` to our compilers, but that is not the case with our current
> Makefile. So I must have been misremembering.
We do for DEVELOPER=1 mode, but it wouldn't make sense to pass something
unportable like "gnu99" for the general case.
-Peff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] C23 compatibility
2024-11-18 7:45 ` Patrick Steinhardt
2024-11-18 9:58 ` Jeff King
@ 2024-11-18 22:11 ` brian m. carlson
1 sibling, 0 replies; 8+ messages in thread
From: brian m. carlson @ 2024-11-18 22:11 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Sam James
[-- Attachment #1: Type: text/plain, Size: 686 bytes --]
On 2024-11-18 at 07:45:03, Patrick Steinhardt wrote:
> Both of the patches look obviously good to me. I was a bit surprised
> that this is required in the first place as I thought we were passing
> `-std=gnu99` to our compilers, but that is not the case with our current
> Makefile. So I must have been misremembering.
We can't do that because FreeBSD's system headers require C11 and won't
work with `-std=gnu99`, in addition to the general unportability of that
construct as mentioned by Peff. There's a comment to that effect in
`config.mak.dev`.
I'm pleased you found the patches acceptable, though.
--
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread