On 12/2/22 1:10 PM, Ævar Arnfjörð Bjarmason wrote:
On Fri, Dec 02 2022, Rose via GitGitGadget wrote:
From: Seija <doremylover123@xxxxxxxxx>
The documentation for pthread_create and pthread_sigmask state that:
"On success, pthread_create() returns 0;
on error, it returns an error number"
As such, we ought to check for an error
by seeing if the output is not 0.
Checking for "less than" is a mistake
as the error code numbers can be greater than 0.
Signed-off-by: Seija <doremylover123@xxxxxxxxx>
---
maintenance: compare output of pthread functions for inequality with 0
The documentation for pthread_create and pthread_sigmask state that "On
success, pthread_create() returns 0; on error, it returns an error
number, and the contents of *thread are undefined."
As such, we ought to check for an error by seeing if the output is not
0, rather than being less than 0, since nothing stops these functions
from returning a positive number.
Signed-off by: Seija doremylover123@xxxxxxxxx
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1389%2FAtariDreams%2Faddress-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1389/AtariDreams/address-v1
Pull-Request: https://github.com/git/git/pull/1389
builtin/fsmonitor--daemon.c | 4 ++--
run-command.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index 6f30a4f93a7..52a08bb3b57 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -1209,7 +1209,7 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state)
* events.
*/
if (pthread_create(&state->listener_thread, NULL,
- fsm_listen__thread_proc, state) < 0) {
+ fsm_listen__thread_proc, state)) {
ipc_server_stop_async(state->ipc_server_data);
err = error(_("could not start fsmonitor listener thread"));
goto cleanup;
@@ -1220,7 +1220,7 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state)
* Start the health thread to watch over our process.
*/
if (pthread_create(&state->health_thread, NULL,
- fsm_health__thread_proc, state) < 0) {
+ fsm_health__thread_proc, state)) {
ipc_server_stop_async(state->ipc_server_data);
err = error(_("could not start fsmonitor health thread"));
goto cleanup;
diff --git a/run-command.c b/run-command.c
index 48b9ba6d6f0..756f1839aab 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1019,7 +1019,7 @@ static void *run_thread(void *data)
sigset_t mask;
sigemptyset(&mask);
sigaddset(&mask, SIGPIPE);
- if (pthread_sigmask(SIG_BLOCK, &mask, NULL) < 0) {
+ if (pthread_sigmask(SIG_BLOCK, &mask, NULL)) {
ret = error("unable to block SIGPIPE in async thread");
return (void *)ret;
}
base-commit: 805265fcf7a737664a8321aaf4a0587b78435184
This looks good to me, and skimming through the rest of the
pthread_create() it seems the rest of the code in-tree is correct.
But (and especially if you're interested) we really should follow-up
here and fix the "error()" etc. part of this. After this we have cases
in-tree where we on failure:
But to be clear, the pthread_ changes are good by themselves and can
be considered a single task that could be advanced without any
extra stuff.
All of the following, if of interest to you or anyone else, should
be done in one or more separate/later and independent series.
* Call die_errno() (good)
* Call die(), error() etc., but with a manual strerror() argument,
these should just use the *_errno() helper.
* Don't report on the errno at all, e.g. in this case shown here.
It seems to me that all of these should be using die_errno(),
error_errno() etc.
Or maybe it's the other way around, and we should not rely on the global
"errno", but always capture the return value, and give that to
strerror() (or set "errno = ret", and call {die,error,warning}_errno()).
In any case, some low-hanging #leftoverbits there...
