coverity: fix unchecked returns

This is the next batch of fixes in response to issues reported by Coverity.

Changes since v2:

* Added a new commit to handle block-writer initialization errors (instead of ignoring them).
* The bw->zstream attribute is now also deinitialized in the error case, as suggested by Junio.
* The commit message of "reftable/block: check deflateInit() return value" was rephrased to stop suggesting that silent corruption by zlib would be possible before that patch: This turned out to be provably incorrect.
* When aborting the bisect because dup2() failed, a left-over saved_stdout is now also cleaned up.

Changes since v1:

* The last-modified patch is now more careful to clean up a commit slab when parsing the commit failed.
* When the "good" bisect term was read successfully, but not the "bad" one, the "good" one is now cleaned up.
* Instead of detecting failed get_terms() calls indirectly, the return value is now checked.
* Failures when bisect_run() calls dup2() are now handled properly, too.

Johannes Schindelin (12):
http: die on curl_easy_duphandle failure in get_active_slot
config: propagate launch_editor() failure in show_editor()
reftable: handle block-writer initialization errors reftable/block: check deflateInit() return value reftable tests: check reftable_table_init_ref_iterator() return
last-modified: handle repo_parse_commit() failures compat/pread: check initial lseek for errors
transport-helper: check dup() return in get_exporter
transport-helper: warn when export-marks file cannot be finalized
bisect: check strbuf_getline_lf return when reading terms
bisect: check get_terms return at all call sites
bisect: handle dup() failure when redirecting stdout

 bisect.c                        |  6 +++--
 builtin/bisect.c                | 44 ++++++++++++++++++++++++---------
 builtin/config.c                |  5 +++-
 builtin/last-modified.c         |  9 ++++---
 compat/pread.c                  |  2 ++
 http.c                          |  2 ++
 reftable/block.c                |  5 +++-
 reftable/writer.c               |  8 +++++-
 t/unit-tests/u-reftable-table.c |  6 +++--
 transport-helper.c              |  6 ++++-
 10 files changed, 70 insertions(+), 23 deletions(-)

base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2179%2Fdscho%2Fcoverity-fixes-unchecked-returns-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2179/dscho/coverity-fixes-unchecked-returns-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2179

Range-diff vs v2:

  1:  e653255de1 =  1:  e653255de1 http: die on curl_easy_duphandle failure in get_active_slot
  2:  0692704d45 =  2:  0692704d45 config: propagate launch_editor() failure in show_editor()
  -:  ---------- >  3:  c689148aef reftable: handle block-writer initialization errors
  3:  9bf7e737c7 !  4:  66953a65d0 reftable/block: check deflateInit() return value

@@ Commit message
z_stream is left in an undefined state.

Subsequent deflate() calls in block_writer_finish() then operate
- on this uninitialized stream. Depending on the zlib
- implementation, this can produce silently corrupted compressed
- data (which would be written to the reftable file and discovered
- only when a later reader fails to inflate) or crash outright.
+ on this uninitialized stream. Current zlib/zlib-ng versions handle
+ such a stream gracefully, by returning Z_STREAM_ERROR, so in
+ practice it would likely not result in catastrophic error.

- The function already uses REFTABLE_ZLIB_ERROR for deflate()
- failures later in the code path (lines 171, 199), so returning
- the same error code for deflateInit() failure is consistent.
+ The function already uses REFTABLE_ZLIB_ERROR for deflate() failures
+ later in the code path, so returning the same error code for
+ deflateInit() failure is consistent.

Pointed out by Coverity.

+ Helped-by: Junio C Hamano <[email protected]>

## reftable/block.c ##
@@ reftable/block.c: int block_writer_init(struct block_writer *bw, uint8_t typ, ui
if (!bw->zstream)
return REFTABLE_OUT_OF_MEMORY_ERROR;
- deflateInit(bw->zstream, 9);
-+ if (deflateInit(bw->zstream, 9) != Z_OK)
++ if (deflateInit(bw->zstream, 9) != Z_OK) {
++ REFTABLE_FREE_AND_NULL(bw->zstream);
+ return REFTABLE_ZLIB_ERROR;
++ }
}

return 0;

  4:  711671c3ab =  5:  a49af20d30 reftable tests: check reftable_table_init_ref_iterator() return
  5:  72a74c76be =  6:  bf06239732 last-modified: handle repo_parse_commit() failures
  6:  f0b1e13979 =  7:  6e2295b8f0 compat/pread: check initial lseek for errors
  7:  0facb9e8ca =  8:  689bb48fe5 transport-helper: check dup() return in get_exporter
  8:  2b0e4f32fd =  9:  ad6ea19737 transport-helper: warn when export-marks file cannot be finalized
  9:  7f2b963103 = 10:  7db6ac2ab0 bisect: check strbuf_getline_lf return when reading terms
 10:  9a9103096a = 11:  aefdbe2bdf bisect: check get_terms return at all call sites
 11:  829cd82177 ! 12:  258dbb0fbd bisect: handle dup() failure when redirecting stdout
     @@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
      +		if (saved_stdout < 0 ||
      +		    dup2(temporary_stdout_fd, 1) < 0) {
      +			res = error_errno(_("could not duplicate stdout"));
     ++			if (saved_stdout >= 0)
     ++				close(saved_stdout);
      +			close(temporary_stdout_fd);
      +			break;
      +		}

gitgitgadget