threads / patch / 44135

patchrun-command: async_exit no longer needs to be public

Subject: [PATCH] run-command: async_exit no longer needs to be public

## tl;dr

6 messages between Sep 22, 2016 and Sep 24, 2016. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Ramsay Jones· Sep 22, 2016, 16:56 UTC · lore
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Lars,

If you need to re-roll your 'ls/filter-process' branch, could you please squash this into the relevant commit c42a4cbc ("run-command: move check_pipe() from write_or_die to run_command", 20-09-2016).

[Note that commit 9658846c ("write_or_die: handle EPIPE in async threads", 24-02-2016) introduced async_exit() specifically for use in the implementation of check_pipe(). Now that you have moved check_pipe() into run-command.c, it no longer needs to be public.]

Thanks!

ATB, Ramsay Jones

 run-command.c | 30 +++++++++++++++---------------
 run-command.h |  3 +--
 2 files changed, 16 insertions(+), 17 deletions(-)
Show changes to 2 files +16 −17

run-command.c, run-command.h

diff --git a/run-command.c b/run-command.c
index b72f6d1..3269362 100644
--- a/run-command.c
+++ b/run-command.c
@@ -6,19 +6,6 @@
 #include "thread-utils.h"
 #include "strbuf.h"
 
-void check_pipe(int err)
-{
-	if (err == EPIPE) {
-		if (in_async())
-			async_exit(141);
-
-		signal(SIGPIPE, SIG_DFL);
-		raise(SIGPIPE);
-		/* Should never happen, but just in case... */
-		exit(141);
-	}
-}
-
 void child_process_init(struct child_process *child)
 {
 	memset(child, 0, sizeof(*child));
@@ -647,7 +634,7 @@ int in_async(void)
 	return !pthread_equal(main_thread, pthread_self());
 }
 
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
 {
 	pthread_exit((void *)(intptr_t)code);
 }
@@ -697,13 +684,26 @@ int in_async(void)
 	return process_is_async;
 }
 
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
 {
 	exit(code);
 }
 
 #endif
 
+void check_pipe(int err)
+{
+	if (err == EPIPE) {
+		if (in_async())
+			async_exit(141);
+
+		signal(SIGPIPE, SIG_DFL);
+		raise(SIGPIPE);
+		/* Should never happen, but just in case... */
+		exit(141);
+	}
+}
+
 int start_async(struct async *async)
 {
 	int need_in, need_out;
diff --git a/run-command.h b/run-command.h
index e7c5f71..bb89c30 100644
--- a/run-command.h
+++ b/run-command.h
@@ -54,7 +54,6 @@ int finish_command(struct child_process *);
 int finish_command_in_signal(struct child_process *);
 int run_command(struct child_process *);
 
-void check_pipe(int err);
 
 /*
  * Returns the path to the hook file, or NULL if the hook is missing
@@ -141,7 +140,7 @@ struct async {
 int start_async(struct async *async);
 int finish_async(struct async *async);
 int in_async(void);
-void NORETURN async_exit(int code);
+void check_pipe(int err);
 
 /**
  * This callback should initialize the child process and preload the

-- 
2.10.0
Lars Schneider· Sep 23, 2016, 08:26 UTC · re: Ramsay Jones · lore

Re: [PATCH] run-command: async_exit no longer needs to be public

Show 16 quoted lines
> On 22 Sep 2016, at 18:56, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
> 
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> 
> Hi Lars,
> 
> If you need to re-roll your 'ls/filter-process' branch, could you please
> squash this into the relevant commit c42a4cbc ("run-command: move check_pipe()
> from write_or_die to run_command", 20-09-2016).
> 
> [Note that commit 9658846c ("write_or_die: handle EPIPE in async threads",
> 24-02-2016) introduced async_exit() specifically for use in the implementation
> of check_pipe(). Now that you have moved check_pipe() into run-command.c,
> it no longer needs to be public.]
Hi Ramsay,

thanks for noticing this. I actually hope that I don't need another re-roll :-) If I don't re-roll. Should I make a patch with this cleanup or do you take care of it?

Thanks, Lars

Junio C Hamano· Sep 23, 2016, 17:13 UTC · re: Lars Schneider · lore

Re: [PATCH] run-command: async_exit no longer needs to be public

Lars Schneider <larsxschneider@gmail.com> writes:
Show 14 quoted lines
>> If you need to re-roll your 'ls/filter-process' branch, could you please
>> squash this into the relevant commit c42a4cbc ("run-command: move check_pipe()
>> from write_or_die to run_command", 20-09-2016).
>> 
>> [Note that commit 9658846c ("write_or_die: handle EPIPE in async threads",
>> 24-02-2016) introduced async_exit() specifically for use in the implementation
>> of check_pipe(). Now that you have moved check_pipe() into run-command.c,
>> it no longer needs to be public.]
>
> Hi Ramsay,
>
> thanks for noticing this. I actually hope that I don't need another re-roll :-)
> If I don't re-roll. Should I make a patch with this cleanup or do you
> take care of it?

I can just squash the the patch you are responding to into c42a4cbc, with an additional paragraph "While at it, retire async_exit() as a public function as it no longer is called outside run-command API implementation", or something like that.

I do not offhand know if the topic is otherwise ready as-is, or needs further work. When you need to reroll, you'd also need to fetch from the result of the above from me first and then start your work from it, though, if we go that route.

Lars Schneider· Sep 23, 2016, 18:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] run-command: async_exit no longer needs to be public

Show 28 quoted lines
> On 23 Sep 2016, at 19:13, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>>> If you need to re-roll your 'ls/filter-process' branch, could you please
>>> squash this into the relevant commit c42a4cbc ("run-command: move check_pipe()
>>> from write_or_die to run_command", 20-09-2016).
>>> 
>>> [Note that commit 9658846c ("write_or_die: handle EPIPE in async threads",
>>> 24-02-2016) introduced async_exit() specifically for use in the implementation
>>> of check_pipe(). Now that you have moved check_pipe() into run-command.c,
>>> it no longer needs to be public.]
>> 
>> Hi Ramsay,
>> 
>> thanks for noticing this. I actually hope that I don't need another re-roll :-)
>> If I don't re-roll. Should I make a patch with this cleanup or do you
>> take care of it?
> 
> I can just squash the the patch you are responding to into c42a4cbc,
> with an additional paragraph "While at it, retire async_exit() as a
> public function as it no longer is called outside run-command API
> implementation", or something like that.
> 
> I do not offhand know if the topic is otherwise ready as-is, or
> needs further work.  When you need to reroll, you'd also need to
> fetch from the result of the above from me first and then start your
> work from it, though, if we go that route.
Sounds good to me!
Thank you, Junio!
Junio C Hamano· Sep 23, 2016, 19:26 UTC · re: Lars Schneider · lore

Re: [PATCH] run-command: async_exit no longer needs to be public

Lars Schneider <larsxschneider@gmail.com> writes:
Show 6 quoted lines
>> I do not offhand know if the topic is otherwise ready as-is, or
>> needs further work.  When you need to reroll, you'd also need to
>> fetch from the result of the above from me first and then start your
>> work from it, though, if we go that route.
>
> Sounds good to me!
OK, here is what I queued, then.
-- >8 --
From: Lars Schneider <larsxschneider@gmail.com>
Date: Tue, 20 Sep 2016 21:02:39 +0200
Subject: [PATCH] run-command: move check_pipe() from write_or_die to
 run_command

Move check_pipe() to run_command and make it public. This is necessary to call the function from pkt-line in a subsequent patch.

While at it, make async_exit() static to run_command.c as it is no longer used from outside.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 run-command.c  | 17 +++++++++++++++--
 run-command.h  |  2 +-
 write_or_die.c | 13 -------------
 3 files changed, 16 insertions(+), 16 deletions(-)
Show changes to 3 files +16 −16

run-command.c, run-command.h, write_or_die.c

diff --git a/run-command.c b/run-command.c
index 5a4dbb6..3269362 100644
--- a/run-command.c
+++ b/run-command.c
@@ -634,7 +634,7 @@ int in_async(void)
 	return !pthread_equal(main_thread, pthread_self());
 }
 
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
 {
 	pthread_exit((void *)(intptr_t)code);
 }
@@ -684,13 +684,26 @@ int in_async(void)
 	return process_is_async;
 }
 
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
 {
 	exit(code);
 }
 
 #endif
 
+void check_pipe(int err)
+{
+	if (err == EPIPE) {
+		if (in_async())
+			async_exit(141);
+
+		signal(SIGPIPE, SIG_DFL);
+		raise(SIGPIPE);
+		/* Should never happen, but just in case... */
+		exit(141);
+	}
+}
+
 int start_async(struct async *async)
 {
 	int need_in, need_out;
diff --git a/run-command.h b/run-command.h
index 5066649..cf29a31 100644
--- a/run-command.h
+++ b/run-command.h
@@ -139,7 +139,7 @@ struct async {
 int start_async(struct async *async);
 int finish_async(struct async *async);
 int in_async(void);
-void NORETURN async_exit(int code);
+void check_pipe(int err);
 
 /**
  * This callback should initialize the child process and preload the
diff --git a/write_or_die.c b/write_or_die.c
index 0734432..eab8c8d 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,19 +1,6 @@
 #include "cache.h"
 #include "run-command.h"
 
-static void check_pipe(int err)
-{
-	if (err == EPIPE) {
-		if (in_async())
-			async_exit(141);
-
-		signal(SIGPIPE, SIG_DFL);
-		raise(SIGPIPE);
-		/* Should never happen, but just in case... */
-		exit(141);
-	}
-}
-
 /*
  * Some cases use stdio, but want to flush after the write
  * to get error handling (and to get better interactive

-- 
2.10.0-530-g67247c9
Ramsay Jones· Sep 24, 2016, 01:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] run-command: async_exit no longer needs to be public

On 23/09/16 20:26, Junio C Hamano wrote:
Show 10 quoted lines
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>>> I do not offhand know if the topic is otherwise ready as-is, or
>>> needs further work.  When you need to reroll, you'd also need to
>>> fetch from the result of the above from me first and then start your
>>> work from it, though, if we go that route.
>>
>> Sounds good to me!
> 
> OK, here is what I queued, then.
This looks good to me. Thanks!

ATB, Ramsay Jones

← back to recent threads