git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: Rose via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Seija <doremylover123@gmail.com>
Subject: Re: [PATCH] maintenance: compare output of pthread functions for inequality with 0
Date: Fri, 02 Dec 2022 23:46:25 +0100	[thread overview]
Message-ID: <221202.86y1rpe7ma.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <Y4pl5BzQnw0Fm+5S@tapette.crustytoothpaste.net>


On Fri, Dec 02 2022, brian m. carlson wrote:

> [[PGP Signed Part:Undecided]]
> On 2022-12-02 at 18:10:57, Ævar Arnfjörð Bjarmason wrote:
>> 
>> 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:
>> 
>>  * 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.
>
> Actually, I don't think that's correct.
>
>> 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()).
>
> Yeah, I think we need to do this.  That's because unlike most other
> functions, the pthread functions _don't_ set errno, and instead return
> the error value.  That's why on a typical Unix system, we would have
> never failed before this patch: because errno values are always
> positive.

I was skimming the POSIX docs earlier, which seem to indicate that
you're not promised anyhting about "errno" being set, just the return
value.

But at the same time I was reading glibc's pthread implementation, where
a lot of the time (but not all the time!) you'll also get errno, just as
an artifact of the library carrying forward an error from an internal
API which failed while setting errno (e.g. malloc()).

In any case, the best thing to do for our codebase is probably:

	if ((errno = pthread_create(...)))
        	die_errno(...);

Since that gives our usag.[ch] library the chance to do something more
clever than doing the same strerror() formatting hardcoded at every
callsite.

  reply	other threads:[~2022-12-02 22:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-02 17:02 [PATCH] maintenance: compare output of pthread functions for inequality with 0 Rose via GitGitGadget
2022-12-02 18:05 ` Jeff Hostetler
2022-12-02 18:10 ` Ævar Arnfjörð Bjarmason
2022-12-02 18:44   ` Jeff Hostetler
2022-12-02 20:55   ` brian m. carlson
2022-12-02 22:46     ` Ævar Arnfjörð Bjarmason [this message]
2022-12-03  0:26       ` brian m. carlson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=221202.86y1rpe7ma.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=doremylover123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).