git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Brandon Williams <bmwill@google.com>,
	Stefan Beller <sbeller@google.com>
Subject: Re: [PATCH 0/2] Fix crashes due to real_pathdup() potentially returning NULL
Date: Wed, 8 Mar 2017 11:17:42 -0500	[thread overview]
Message-ID: <20170308161742.lpbf6gn6qhoex7et@sigill.intra.peff.net> (raw)
In-Reply-To: <cover.1488987786.git.johannes.schindelin@gmx.de>

On Wed, Mar 08, 2017 at 04:43:27PM +0100, Johannes Schindelin wrote:

> We may want to consider fast-tracking this into v2.12.1, and to that
> end, I would appreciate code reviews that focus on the correctness of
> this patch and that try to consider undesired side effects.

I don't see how it could be not-correct, in the sense that every caller
now passes the die_on_error flag (restoring the original behavior)
except for the one which clearly checks for a NULL return immediately
afterward.

The only exception would be if there were new calls to real_pathdup()
that did not originally use real_path(). But:

  # 7241764076 introduced real_pathdup
  git log -Sreal_pathdup 7241764076..

shows only one other introduction, and it's just duplicating an existing
call.

It's possible that some of these _could_ handle the error case more
gracefully (I already fixed one such case in 3a1345af2). But even if
we wanted to do so, that should come separately on top of this patch.
This can go to 'maint' as a regression fix, and then that gives a stable
base for making potential improvements.

-Peff

  parent reply	other threads:[~2017-03-08 16:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08 15:43 [PATCH 0/2] Fix crashes due to real_pathdup() potentially returning NULL Johannes Schindelin
2017-03-08 15:43 ` [PATCH 1/2] Demonstrate NULL pointer access with invalid GIT_WORK_TREE Johannes Schindelin
2017-03-08 15:43 ` [PATCH 2/2] Fix callsites of real_pathdup() that wanted it to die on error Johannes Schindelin
2017-03-08 18:12   ` René Scharfe
2017-03-08 18:38     ` Brandon Williams
2017-03-08 21:16       ` Junio C Hamano
2017-03-09 11:24         ` Johannes Schindelin
2017-03-09 16:33           ` René Scharfe
2017-03-08 16:17 ` Jeff King [this message]
2017-03-09 11:26   ` [PATCH 0/2] Fix crashes due to real_pathdup() potentially returning NULL Johannes Schindelin

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=20170308161742.lpbf6gn6qhoex7et@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=sbeller@google.com \
    /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).