Re: [PATCH 0/2] Fix crashes due to real_pathdup() potentially returning NULL

From: Jeff King <peff@peff.net>
Date: Mar 8, 2017, 16:17 UTC
Message-ID: <20170308161742.lpbf6gn6qhoex7et@sigill.intra.peff.net>
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