git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: gitgitgadget@gmail.com
Cc: git@vger.kernel.org, hanwenn@gmail.com,
	Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH 0/8] refs: cleanup errno sideband ref related functions
Date: Wed,  2 Jun 2021 19:13:56 -0700	[thread overview]
Message-ID: <20210603021356.342665-1-jonathantanmy@google.com> (raw)
In-Reply-To: <pull.1012.git.git.1619710329.gitgitgadget@gmail.com>

> v3
> 
>  * remove errno as an implicit communication mechanism from refs support
>    completely.

Looking at all the patches, it seems that this patch set is about
functions in Git code that set errno themselves to indicate the category
of failure encountered, and instead of setting errno, we want them to
transmit that information through an out param instead. I notice that
the cover letter talks about "cleanup" and "remove errno", but that
could have been explained in greater detail, I think.

As for whether it is a good idea overall, it could be said that errno is
idiomatic in C and writing "if (myfunc()) { store_errno = errno; ..." is
not much more difficult than "int store_errno; if (myfunc(&store_errno))
{ ...". But presumably Han-Wen thinks the opposite, since he wrote this
patch set, and I'll defer to his opinion on this since he's working on
the ref code and I'm not. Besides that, having the out param is more
explicit (which might be better) and permits chaining of such functions
(e.g. "if (myfunc(&store_errno) || myotherfunc(&store_errno))"). I'll
review this patch set as if this is the approach we want to take.

  parent reply	other threads:[~2021-06-03  2:14 UTC|newest]

Thread overview: 137+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-29 15:32 Han-Wen Nienhuys via GitGitGadget
2021-04-29 15:32 ` [PATCH 1/8] refs: remove EINVAL specification from the errno sideband in read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-04-30  2:38   ` Junio C Hamano
2021-05-19 12:25     ` Han-Wen Nienhuys
2021-06-03  2:19   ` Jonathan Tan
2021-06-09 11:28     ` Han-Wen Nienhuys
2021-04-29 15:32 ` [PATCH 2/8] refs/files-backend: stop setting errno from lock_ref_oid_basic Han-Wen Nienhuys via GitGitGadget
2021-04-30  3:10   ` Junio C Hamano
2021-05-19 12:29     ` Han-Wen Nienhuys
2021-06-03  2:33   ` Jonathan Tan
2021-06-10 10:02     ` Han-Wen Nienhuys
2021-04-29 15:32 ` [PATCH 3/8] refs: make errno output explicit for read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-04-30  3:34   ` Junio C Hamano
2021-04-30  6:02     ` Junio C Hamano
2021-05-19 12:33       ` Han-Wen Nienhuys
2021-06-03  2:37   ` Jonathan Tan
2021-06-10 10:05     ` Han-Wen Nienhuys
2021-04-29 15:32 ` [PATCH 4/8] refs: make errno output explicit for refs_resolve_ref_unsafe Han-Wen Nienhuys via GitGitGadget
2021-06-03  2:51   ` Jonathan Tan
2021-06-10 11:27     ` Han-Wen Nienhuys
2021-04-29 15:32 ` [PATCH 5/8] refs: add failure_errno to refs_read_raw_ref() signature Han-Wen Nienhuys via GitGitGadget
2021-04-29 15:32 ` [PATCH 6/8] refs: clear errno return in refs_resolve_ref_unsafe() Han-Wen Nienhuys via GitGitGadget
2021-06-03  2:53   ` Jonathan Tan
2021-06-10 11:45     ` Han-Wen Nienhuys
2021-04-29 15:32 ` [PATCH 7/8] refs: stop setting EINVAL and ELOOP in symref resolution Han-Wen Nienhuys via GitGitGadget
2021-06-03  2:55   ` Jonathan Tan
2021-06-10 11:58     ` Han-Wen Nienhuys
2021-04-29 15:32 ` [PATCH 8/8] refs: explicitly propagate errno from refs_read_raw_ref Han-Wen Nienhuys via GitGitGadget
2021-06-03  2:13 ` Jonathan Tan [this message]
2021-06-09 11:29   ` [PATCH 0/8] refs: cleanup errno sideband ref related functions Han-Wen Nienhuys
2021-06-10 12:57 ` [PATCH v2 " Han-Wen Nienhuys via GitGitGadget
2021-06-10 12:57   ` [PATCH v2 1/8] refs: remove EINVAL errno output from specification of read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-06-10 12:57   ` [PATCH v2 2/8] refs/files-backend: stop setting errno from lock_ref_oid_basic Han-Wen Nienhuys via GitGitGadget
2021-07-01 11:13     ` Ævar Arnfjörð Bjarmason
2021-07-05 14:16       ` Han-Wen Nienhuys
2021-06-10 12:57   ` [PATCH v2 3/8] refs: make errno output explicit for read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-07-01 11:34     ` Ævar Arnfjörð Bjarmason
2021-07-05 14:34       ` Han-Wen Nienhuys
2021-06-10 12:57   ` [PATCH v2 4/8] refs: make errno output explicit for refs_resolve_ref_unsafe Han-Wen Nienhuys via GitGitGadget
2021-07-01 11:56     ` Ævar Arnfjörð Bjarmason
2021-06-10 12:57   ` [PATCH v2 5/8] refs: use refs_resolve_ref_unsafe_with_errno() where needed Han-Wen Nienhuys via GitGitGadget
2021-07-01 11:58     ` Ævar Arnfjörð Bjarmason
2021-06-10 12:57   ` [PATCH v2 6/8] refs: add failure_errno to refs_read_raw_ref() signature Han-Wen Nienhuys via GitGitGadget
2021-07-01 12:06     ` Ævar Arnfjörð Bjarmason
2021-06-10 12:57   ` [PATCH v2 7/8] refs: clear errno return in refs_resolve_ref_unsafe() Han-Wen Nienhuys via GitGitGadget
2021-07-01 12:19     ` Ævar Arnfjörð Bjarmason
2021-06-10 12:57   ` [PATCH v2 8/8] refs: explicitly propagate errno from refs_read_raw_ref Han-Wen Nienhuys via GitGitGadget
2021-07-01 12:26     ` Ævar Arnfjörð Bjarmason
2021-07-05 16:09       ` Han-Wen Nienhuys
2021-07-05 19:08         ` Ævar Arnfjörð Bjarmason
2021-07-05 19:39           ` Han-Wen Nienhuys
2021-06-14 10:10   ` [PATCH v2 0/8] refs: cleanup errno sideband ref related functions Han-Wen Nienhuys
2021-07-05 20:56   ` [PATCH v3 0/5] " Han-Wen Nienhuys via GitGitGadget
2021-07-05 20:56     ` [PATCH v3 1/5] refs: remove EINVAL errno output from specification of read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-07-05 20:56     ` [PATCH v3 2/5] refs/files-backend: stop setting errno from lock_ref_oid_basic Han-Wen Nienhuys via GitGitGadget
2021-07-05 20:56     ` [PATCH v3 3/5] refs: make errno output explicit for read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-07-05 20:56     ` [PATCH v3 4/5] refs: add failure_errno to refs_read_raw_ref() signature Han-Wen Nienhuys via GitGitGadget
2021-07-06 19:28       ` Junio C Hamano
2021-07-05 20:56     ` [PATCH v3 5/5] refs: make errno output explicit for refs_resolve_ref_unsafe Han-Wen Nienhuys via GitGitGadget
2021-07-06  0:38     ` [PATCH v3 0/5] refs: cleanup errno sideband ref related functions Ævar Arnfjörð Bjarmason
2021-07-06  9:53       ` Han-Wen Nienhuys
2021-07-06 14:27         ` Ævar Arnfjörð Bjarmason
2021-07-06 18:36           ` Han-Wen Nienhuys
2021-07-06 18:55     ` [PATCH v4 0/6] " Han-Wen Nienhuys via GitGitGadget
2021-07-06 18:55       ` [PATCH v4 1/6] refs: remove EINVAL errno output from specification of read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-07-06 18:55       ` [PATCH v4 2/6] refs/files-backend: stop setting errno from lock_ref_oid_basic Han-Wen Nienhuys via GitGitGadget
2021-07-06 18:55       ` [PATCH v4 3/6] refs: make errno output explicit for read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-07-06 18:55       ` [PATCH v4 4/6] refs: add failure_errno to refs_read_raw_ref() signature Han-Wen Nienhuys via GitGitGadget
2021-07-06 19:39         ` Junio C Hamano
2021-07-06 18:55       ` [PATCH v4 5/6] refs: explicitly return failure_errno from parse_loose_ref_contents Han-Wen Nienhuys via GitGitGadget
2021-07-06 19:37         ` Junio C Hamano
2021-07-07  8:20           ` Han-Wen Nienhuys
2021-07-06 18:55       ` [PATCH v4 6/6] refs: make errno output explicit for refs_resolve_ref_unsafe Han-Wen Nienhuys via GitGitGadget
2021-07-07 19:07       ` [PATCH v5 0/6] refs: cleanup errno sideband ref related functions Han-Wen Nienhuys via GitGitGadget
2021-07-07 19:07         ` [PATCH v5 1/6] refs: remove EINVAL errno output from specification of read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-07-07 19:07         ` [PATCH v5 2/6] refs/files-backend: stop setting errno from lock_ref_oid_basic Han-Wen Nienhuys via GitGitGadget
2021-07-11 11:38           ` Ævar Arnfjörð Bjarmason
2021-07-13  8:00             ` Han-Wen Nienhuys
2021-07-07 19:07         ` [PATCH v5 3/6] refs: make errno output explicit for read_raw_ref_fn Han-Wen Nienhuys via GitGitGadget
2021-07-11 11:52           ` Ævar Arnfjörð Bjarmason
2021-07-07 19:07         ` [PATCH v5 4/6] refs: add failure_errno to refs_read_raw_ref() signature Han-Wen Nienhuys via GitGitGadget
2021-07-11 11:59           ` Ævar Arnfjörð Bjarmason
2021-07-13  8:02             ` Han-Wen Nienhuys
2021-07-07 19:07         ` [PATCH v5 5/6] refs: explicitly return failure_errno from parse_loose_ref_contents Han-Wen Nienhuys via GitGitGadget
2021-07-11 12:41           ` Ævar Arnfjörð Bjarmason
2021-07-07 19:07         ` [PATCH v5 6/6] refs: make errno output explicit for refs_resolve_ref_unsafe Han-Wen Nienhuys via GitGitGadget
2021-07-07 20:44         ` [PATCH v5 0/6] refs: cleanup errno sideband ref related functions Junio C Hamano
2021-07-11 16:30         ` [PATCH v6? 00/17] refs API: get rid of errno setting entirely Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 01/17] refs: remove EINVAL errno output from specification of read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 02/17] refs/files-backend: stop setting errno from lock_ref_oid_basic Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 03/17] refs: make errno output explicit for read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 04/17] refs: add failure_errno to refs_read_raw_ref() signature Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 05/17] refs: explicitly return failure_errno from parse_loose_ref_contents Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 06/17] refs: make errno output explicit for refs_resolve_ref_unsafe Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 07/17] refs: make errno ignoring explicit in lock_ref_oid_basic() Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 08/17] refs file-backend.c: stop setting "EBUSY" in verify_lock() Ævar Arnfjörð Bjarmason
2021-07-13  8:08             ` Han-Wen Nienhuys
2021-07-11 16:30           ` [PATCH v6? 09/17] refs file-backend.c: deal with errno directly " Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 10/17] refs API: remove refs_read_ref_full() wrapper Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 11/17] refs API: make resolve_gitlink_ref() not set errno Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 12/17] refs API: make refs_resolve_ref_unsafe() static Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 13/17] refs API: make refs_resolve_refdup() not set errno Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 14/17] refs API: make refs_ref_exists() " Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 15/17] refs API: make resolve_ref_unsafe() " Ævar Arnfjörð Bjarmason
2021-07-13  8:13             ` Han-Wen Nienhuys
2021-07-14  8:32               ` Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 16/17] refs API: make expand_ref() and repo_dwim_log() " Ævar Arnfjörð Bjarmason
2021-07-11 16:30           ` [PATCH v6? 17/17] refs API: don't leak "errno" in run_transaction_hook() Ævar Arnfjörð Bjarmason
2021-07-13  8:28           ` [PATCH v6? 00/17] refs API: get rid of errno setting entirely Han-Wen Nienhuys
2021-07-13 18:26             ` Ævar Arnfjörð Bjarmason
2021-07-14  8:38               ` Ævar Arnfjörð Bjarmason
2021-07-14  8:43                 ` Han-Wen Nienhuys
2021-07-14 11:43           ` [PATCH v7 0/6] refs: cleanup errno sideband ref related functions Ævar Arnfjörð Bjarmason
2021-07-14 11:43             ` [PATCH v7 1/6] refs: remove EINVAL errno output from specification of read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-14 11:43             ` [PATCH v7 2/6] refs/files-backend: stop setting errno from lock_ref_oid_basic Ævar Arnfjörð Bjarmason
2021-07-14 11:43             ` [PATCH v7 3/6] refs: make errno output explicit for read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-14 11:43             ` [PATCH v7 4/6] refs: add failure_errno to refs_read_raw_ref() signature Ævar Arnfjörð Bjarmason
2021-07-14 11:43             ` [PATCH v7 5/6] refs: explicitly return failure_errno from parse_loose_ref_contents Ævar Arnfjörð Bjarmason
2021-07-14 11:43             ` [PATCH v7 6/6] refs: make errno output explicit for refs_resolve_ref_unsafe Ævar Arnfjörð Bjarmason
2021-07-16 14:22             ` [PATCH v8 0/7] refs: cleanup errno sideband ref related functions Ævar Arnfjörð Bjarmason
2021-07-16 14:22               ` [PATCH v8 1/7] refs file backend: move raceproof_create_file() here Ævar Arnfjörð Bjarmason
2021-07-16 14:22               ` [PATCH v8 2/7] refs: remove EINVAL errno output from specification of read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-16 14:22               ` [PATCH v8 3/7] refs/files-backend: stop setting errno from lock_ref_oid_basic Ævar Arnfjörð Bjarmason
2021-07-16 14:22               ` [PATCH v8 4/7] refs: make errno output explicit for read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-16 14:22               ` [PATCH v8 5/7] refs: add failure_errno to refs_read_raw_ref() signature Ævar Arnfjörð Bjarmason
2021-07-16 14:22               ` [PATCH v8 6/7] refs: explicitly return failure_errno from parse_loose_ref_contents Ævar Arnfjörð Bjarmason
2021-07-16 14:22               ` [PATCH v8 7/7] refs: make errno output explicit for refs_resolve_ref_unsafe Ævar Arnfjörð Bjarmason
2021-07-20 10:33               ` [PATCH v9 0/7] refs: cleanup errno sideband ref related functions Ævar Arnfjörð Bjarmason
2021-07-20 10:33                 ` [PATCH v9 1/7] refs file backend: move raceproof_create_file() here Ævar Arnfjörð Bjarmason
2021-07-20 10:33                 ` [PATCH v9 2/7] refs: remove EINVAL errno output from specification of read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-20 10:33                 ` [PATCH v9 3/7] refs/files-backend: stop setting errno from lock_ref_oid_basic Ævar Arnfjörð Bjarmason
2021-07-20 10:33                 ` [PATCH v9 4/7] refs: make errno output explicit for read_raw_ref_fn Ævar Arnfjörð Bjarmason
2021-07-20 10:33                 ` [PATCH v9 5/7] refs: add failure_errno to refs_read_raw_ref() signature Ævar Arnfjörð Bjarmason
2021-07-20 10:33                 ` [PATCH v9 6/7] refs: explicitly return failure_errno from parse_loose_ref_contents Ævar Arnfjörð Bjarmason
2021-07-20 10:33                 ` [PATCH v9 7/7] refs: make errno output explicit for refs_resolve_ref_unsafe Ævar Arnfjörð Bjarmason
2021-07-26 23:49                 ` [PATCH v9 0/7] refs: cleanup errno sideband ref related functions Ævar Arnfjörð Bjarmason
2021-07-27  0:18                   ` Junio C Hamano

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=20210603021356.342665-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwenn@gmail.com \
    --subject='Re: [PATCH 0/8] refs: cleanup errno sideband ref related functions' \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

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

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git