Skip to contentSkip to conversation

run-command: fix an 'different modifiers' sparse warning

PATCH In review Ramsay Jones 3 participants 4 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 run-command: fix an 'different modifiers' sparse warning

---

Hi Jeff,

If you need to re-roll your 'jk/epipe-in-async' branch, could you please squash this into the relevant patch. (ie. "write_or_die:
handle EPIPE in async threads", 24-02-2016).

Thanks!

ATB,
Ramsay Jones

 run-command.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
run-command.c +2 -2
index cd861bc..5dec18b 100644
@@ -625,7 +625,7 @@ int in_async(void)
 	return !pthread_equal(main_thread, pthread_self());
 }
 
-void async_exit(int code)
+void NORETURN async_exit(int code)
 {
 	pthread_exit((void *)(intptr_t)code);
 }
@@ -675,7 +675,7 @@ int in_async(void)
 	return process_is_async;
 }
 
-int async_exit(int code)
+int NORETURN async_exit(int code)
 {
 	exit(code);
 }
Signature · 1 line
2.7.0
#1 lore raw
Quoted 10 lines · Ramsay Jones

---

Hi Jeff,

If you need to re-roll your 'jk/epipe-in-async' branch, could you please squash this into the relevant patch. (ie. "write_or_die:
handle EPIPE in async threads", 24-02-2016).

Thanks!

I actually was planning to merge this to 'next' today, so I'll squash it in without waiting for a reroll.

By the way, doesn't it bother anybody to give two different types to the same function depending on NO_PTHREAD? It is not a new issue added by this series, but async_exit() that claims to return int does not (naturally) return anything, and sparse does not seem to care (neither do we).

Quoted 29 lines

ATB,
Ramsay Jones

 run-command.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
run-command.c +2 -2
index cd861bc..5dec18b 100644
@@ -625,7 +625,7 @@ int in_async(void)
 	return !pthread_equal(main_thread, pthread_self());
 }
 
-void async_exit(int code)
+void NORETURN async_exit(int code)
 {
 	pthread_exit((void *)(intptr_t)code);
 }
@@ -675,7 +675,7 @@ int in_async(void)
 	return process_is_async;
 }
 
-int async_exit(int code)
+int NORETURN async_exit(int code)
 {
 	exit(code);
 }
#2 lore raw
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Ramsay Jones

---

Hi Jeff,

Quoted 8 lines
Quoted 5 lines

If you need to re-roll your 'jk/epipe-in-async' branch, could you please squash this into the relevant patch. (ie. "write_or_die:
handle EPIPE in async threads", 24-02-2016).

Thanks!

I actually was planning to merge this to 'next' today, so I'll squash it in without waiting for a reroll.

I am OK with that. But I do find it interesting that we must mark NORETURN in both the declaration and the definition, but we don't for __attribute__((format)).

Quoted 5 lines

By the way, doesn't it bother anybody to give two different types to the same function depending on NO_PTHREAD? It is not a new issue added by this series, but async_exit() that claims to return int does not (naturally) return anything, and sparse does not seem to care (neither do we).

It would have bothered me if I had noticed. :)

It is simply a bug, and sparse (and the compiler) do not notice it because it only shows up if you compile with NO_PTHREADS=1. And I think it is added by this series:

Quoted 6 lines

run-command.c int in_async(void) · line 678

Quoted 6 lines
run-command.c +1 -1
@@ -675,7 +675,7 @@ int in_async(void)
 	return process_is_async;
 }
 
-int async_exit(int code)
+int NORETURN async_exit(int code)

The return value on this one should be "void", too, of course.

-Peff

#3 lore raw
Quoted 28 lines · Jeff King
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Ramsay Jones

---

Hi Jeff,

Quoted 8 lines
Quoted 5 lines

If you need to re-roll your 'jk/epipe-in-async' branch, could you please squash this into the relevant patch. (ie. "write_or_die:
handle EPIPE in async threads", 24-02-2016).

Thanks!

I actually was planning to merge this to 'next' today, so I'll squash it in without waiting for a reroll.

I am OK with that. But I do find it interesting that we must mark NORETURN in both the declaration and the definition, but we don't for __attribute__((format)).

Quoted 5 lines

By the way, doesn't it bother anybody to give two different types to the same function depending on NO_PTHREAD? It is not a new issue added by this series, but async_exit() that claims to return int does not (naturally) return anything, and sparse does not seem to care (neither do we).

Quoted 5 lines

It would have bothered me if I had noticed. :)

It is simply a bug, and sparse (and the compiler) do not notice it because it only shows up if you compile with NO_PTHREADS=1. And I think it is added by this series:

Wow, I can't believe that I didn't notice this myself while editing the file! :-D

ATB,
Ramsay Jones