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: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Enzo Matsumiya" <ematsumiya@suse.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 0/5] run-command API: get rid of "argv"
Date: Mon, 22 Nov 2021 17:04:02 +0100	[thread overview]
Message-ID: <cover-0.5-00000000000-20211122T153605Z-avarab@gmail.com> (raw)
In-Reply-To: <YZseJ4jOVIK3+bUD@coredump.intra.peff.net>

This series is an alternate but more thorough way to solve the pager
segfault reported by Enzo Matsumiya[1], and more generally avoids
similar issues in the future.

That the run-command API exposed two subtly different ways of doing
the same thing wouldn't only lead to the sort of bug reported in [1],
but also made memory management around it rather painful. As noted by
Jeff King in[2]:

    I'd like to eventually get rid of the argv interface entirely
    because it has memory-ownership semantics that are easy to get
    wrong.

There's probably never going to be a perfect time to do this as a
change to this widely used API will probably impact things
in-flight.

Now seems to be a particularly good time though, merging this to
"seen" only conflicts with my ab/config-based-hooks-2 in
builtin/worktree.c. The resolution is trivial (just use the new hook
API added in that topic).

Since this series removes the "argv" member we're not going to have
any semantic conflicts that aren't obvious as a compile-time error
(and merging with seen both compiles & passes all tests).

As noted in 5/5 we've still got a similar issue with "env" and
"env_array". I've got a follow-up series that similarly removes "env"
which we can do at some point (it's much smaller than this one), but
for now let's focus on "argv".

1. https://lore.kernel.org/git/20211120194048.12125-1-ematsumiya@suse.de/
2. https://lore.kernel.org/git/YT6BnnXeAWn8BycF@coredump.intra.peff.net/

Ævar Arnfjörð Bjarmason (5):
  archive-tar: use our own cmd.buf in error message
  upload-archive: use regular "struct child_process" pattern
  run-command API users: use strvec_pushv(), not argv assignment
  run-command API users: use strvec_pushl(), not argv construction
  run-command API: remove "argv" member, always use "args"

 add-patch.c                 |  4 +--
 archive-tar.c               |  9 +++----
 builtin/add.c               |  6 +----
 builtin/fsck.c              | 12 +++------
 builtin/help.c              |  3 +--
 builtin/merge.c             |  3 +--
 builtin/notes.c             |  2 +-
 builtin/receive-pack.c      | 16 +++++------
 builtin/replace.c           |  3 +--
 builtin/upload-archive.c    |  5 +++-
 builtin/worktree.c          |  2 --
 daemon.c                    | 17 +++---------
 diff.c                      |  7 +----
 editor.c                    |  2 +-
 http-backend.c              |  2 +-
 http.c                      |  5 ++--
 prompt.c                    |  7 +----
 remote-curl.c               |  2 +-
 run-command.c               | 53 ++++++++++++++++++++-----------------
 run-command.h               | 20 ++++++--------
 sequencer.c                 |  2 +-
 sub-process.c               |  2 +-
 t/helper/test-run-command.c | 10 ++++---
 t/helper/test-subprocess.c  |  2 +-
 t/t7006-pager.sh            |  4 +++
 trace2/tr2_tgt_event.c      |  2 +-
 trace2/tr2_tgt_normal.c     |  2 +-
 trace2/tr2_tgt_perf.c       |  4 +--
 transport.c                 |  2 +-
 upload-pack.c               |  5 +---
 30 files changed, 94 insertions(+), 121 deletions(-)

-- 
2.34.0.822.gb876f875f1b


  parent reply	other threads:[~2021-11-22 16:04 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-20 19:40 [PATCH v2] pager: fix crash when pager program doesn't exist Enzo Matsumiya
2021-11-21 18:37 ` Jeff King
2021-11-22  2:10   ` Junio C Hamano
2021-11-22  4:35     ` Jeff King
2021-11-22 14:52       ` Enzo Matsumiya
2021-11-22 17:05         ` Junio C Hamano
2021-11-23 16:40           ` Enzo Matsumiya
2021-11-24  1:55             ` Ævar Arnfjörð Bjarmason
2021-11-24 15:51               ` Jeff King
2021-11-22 16:04       ` Ævar Arnfjörð Bjarmason [this message]
2021-11-22 16:04         ` [PATCH 1/5] archive-tar: use our own cmd.buf in error message Ævar Arnfjörð Bjarmason
2021-11-22 21:04           ` Junio C Hamano
2021-11-22 16:04         ` [PATCH 2/5] upload-archive: use regular "struct child_process" pattern Ævar Arnfjörð Bjarmason
2021-11-22 17:02           ` Jeff King
2021-11-22 20:53           ` Ævar Arnfjörð Bjarmason
2021-11-22 21:10             ` Jeff King
2021-11-22 21:36               ` Ævar Arnfjörð Bjarmason
2021-11-22 16:04         ` [PATCH 3/5] run-command API users: use strvec_pushv(), not argv assignment Ævar Arnfjörð Bjarmason
2021-11-22 21:19           ` Junio C Hamano
2021-11-22 21:30             ` Ævar Arnfjörð Bjarmason
2021-11-22 16:04         ` [PATCH 4/5] run-command API users: use strvec_pushl(), not argv construction Ævar Arnfjörð Bjarmason
2021-11-22 16:04         ` [PATCH 5/5] run-command API: remove "argv" member, always use "args" Ævar Arnfjörð Bjarmason
2021-11-22 17:32           ` Jeff King
2021-11-22 18:19             ` Ævar Arnfjörð Bjarmason
2021-11-22 18:47               ` Jeff King
2021-11-22 17:52         ` [PATCH 0/5] run-command API: get rid of "argv" Jeff King
2021-11-22 18:11           ` Junio C Hamano
2021-11-22 18:33             ` Ævar Arnfjörð Bjarmason
2021-11-22 18:49               ` Jeff King
2021-11-22 18:26           ` Ævar Arnfjörð Bjarmason
2021-11-23 12:06         ` [PATCH v2 0/9] run-command API: get rid of "argv" and "env" Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 1/9] worktree: remove redundant NULL-ing of "cp.argv Ævar Arnfjörð Bjarmason
2021-11-23 15:26             ` Eric Sunshine
2021-11-24  1:54               ` Junio C Hamano
2021-11-24  6:00                 ` Eric Sunshine
2021-11-24  6:12                   ` Eric Sunshine
2021-11-24  5:44               ` Eric Sunshine
2021-11-23 12:06           ` [PATCH v2 2/9] upload-archive: use regular "struct child_process" pattern Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 3/9] run-command API users: use strvec_pushv(), not argv assignment Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 4/9] run-command tests: " Ævar Arnfjörð Bjarmason
2021-11-24  1:33             ` Eric Sunshine
2021-11-23 12:06           ` [PATCH v2 5/9] run-command API users: use strvec_pushl(), not argv construction Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 6/9] run-command API users: use strvec_push(), " Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 7/9] run-command API: remove "argv" member, always use "args" Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 8/9] difftool: use "env_array" to simplify memory management Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 9/9] run-command API: remove "env" member, always use "env_array" Ævar Arnfjörð Bjarmason
2021-11-25 22:52           ` [PATCH v3 0/9] run-command API: get rid of "argv" and "env" Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 1/9] worktree: stop being overly intimate with run_command() internals Ævar Arnfjörð Bjarmason
2021-11-26  9:48               ` Eric Sunshine
2021-11-25 22:52             ` [PATCH v3 2/9] upload-archive: use regular "struct child_process" pattern Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 3/9] run-command API users: use strvec_pushv(), not argv assignment Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 4/9] run-command tests: " Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 5/9] run-command API users: use strvec_pushl(), not argv construction Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 6/9] run-command API users: use strvec_push(), " Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 7/9] run-command API: remove "argv" member, always use "args" Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 8/9] difftool: use "env_array" to simplify memory management Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 9/9] run-command API: remove "env" member, always use "env_array" Ævar Arnfjörð Bjarmason
2021-11-22 15:31     ` [PATCH v2] pager: fix crash when pager program doesn't exist Enzo Matsumiya
2021-11-22 16:22       ` Ævar Arnfjörð Bjarmason
2021-11-22 16:46         ` Enzo Matsumiya
2021-11-22 17:10           ` Ævar Arnfjörð Bjarmason
2021-11-22 17:41             ` Jeff King
2021-11-22 18:00             ` Junio C Hamano
2021-11-22 18:26               ` Jeff King
2021-11-22 17:55           ` Junio C Hamano
2021-11-22 18:19             ` Junio C Hamano
2021-11-22 18:37               ` Jeff King
2021-11-22 20:39                 ` Junio C Hamano
2021-11-22 17:08         ` Junio C Hamano
2021-11-22 18:35           ` Ævar Arnfjörð Bjarmason
2021-11-22 16:30       ` Jeff King

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=cover-0.5-00000000000-20211122T153605Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=ematsumiya@suse.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.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).