[v6,00/15] Reftable coverity fixes

Message

John Passaro via GitGitGadget Jan. 20, 2022, 3:11 p.m. UTC

This series was targeted to 'master'.

This addresses some complaints from Coverity that Peff reported.

v6:

 * drop 0-length memcpy paranoia patch.

Han-Wen Nienhuys (15):
  reftable: fix OOB stack write in print functions
  reftable: fix resource leak in block.c error path
  reftable: fix resource leak blocksource.c
  reftable: check reftable_stack_auto_compact() return value
  reftable: ignore remove() return value in stack_test.c
  reftable: fix resource warning
  reftable: all xxx_free() functions accept NULL arguments
  reftable: order unittests by complexity
  reftable: drop stray printf in readwrite_test
  reftable: handle null refnames in reftable_ref_record_equal
  reftable: make reftable-record.h function signatures const correct
  reftable: implement record equality generically
  reftable: remove outdated file reftable.c
  reftable: make reftable_record a tagged union
  reftable: add print functions to the record types

 reftable/block.c           |  28 ++-
 reftable/block_test.c      |  22 +--
 reftable/blocksource.c     |   6 +-
 reftable/generic.c         |  41 +++--
 reftable/iter.c            |   4 +-
 reftable/merged.c          |  33 ++--
 reftable/pq.c              |   3 +-
 reftable/pq_test.c         |  27 ++-
 reftable/reader.c          | 112 ++++++------
 reftable/readwrite_test.c  |  67 ++++++-
 reftable/record.c          | 366 ++++++++++++++++++++++++-------------
 reftable/record.h          |  49 ++---
 reftable/record_test.c     | 197 ++++++++++----------
 reftable/reftable-record.h |  14 +-
 reftable/reftable.c        | 115 ------------
 reftable/stack.c           |  10 +-
 reftable/stack_test.c      |   3 +-
 reftable/writer.c          |  43 +++--
 t/helper/test-reftable.c   |   9 +-
 19 files changed, 620 insertions(+), 529 deletions(-)
 delete mode 100644 reftable/reftable.c


base-commit: 50b2d72e110cad39ecaf2322bfdf1b60cd13dd96
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1152%2Fhanwen%2Freftable-coverity-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1152/hanwen/reftable-coverity-v6
Pull-Request: https://github.com/git/git/pull/1152

Range-diff vs v5:

  1:  e7f1be7bbec =  1:  9218bd59b25 reftable: fix OOB stack write in print functions
  2:  9ab631a3b29 !  2:  315ce62e710 reftable: fix resource leak in block.c error path
     @@ reftable/reader.c: int reader_init_block_reader(struct reftable_reader *r, struc
      +	err = block_reader_init(br, &block, header_off, r->block_size,
      +				hash_size(r->hash_id));
      +done:
     -+	if (err)
     -+		reftable_block_done(&block);
     ++	reftable_block_done(&block);
      +
      +	return err;
       }
     @@ reftable/readwrite_test.c: static void test_log_write_read(void)
      +		},
      +	};
      +
     -+	for (i = 0; i < sizeof(message)-1; i++)
     ++	for (i = 0; i < sizeof(message) - 1; i++)
      +		message[i] = (uint8_t)(rand() % 64 + ' ');
      +
      +	reftable_writer_set_limits(w, 1, 1);
  3:  8301000e83b !  3:  7cf65e141cf reftable: fix resource leak blocksource.c
     @@ Metadata
       ## Commit message ##
          reftable: fix resource leak blocksource.c
      
     -    This would be triggered in the unlikely event of fstat() failing on an opened
     -    file.
     +    This would be triggered in the unlikely event of fstat() failing on an
     +    opened file.
      
          Signed-off-by: Han-Wen Nienhuys <[e-mail]>
      
  4:  e6e53aabc39 =  4:  8740ba92981 reftable: check reftable_stack_auto_compact() return value
  5:  7551bcdd917 =  5:  08be6d90a48 reftable: ignore remove() return value in stack_test.c
  6:  700a4e247e7 !  6:  630f67bdb51 reftable: fix resource warning
     @@ Metadata
       ## Commit message ##
          reftable: fix resource warning
      
     -    This would trigger in the unlikely event that we are compacting, and the next
     -    available file handle is 0.
     +    This would trigger in the unlikely event that we are compacting, and
     +    the next available file handle is 0.
      
          Signed-off-by: Han-Wen Nienhuys <[e-mail]>
      
     @@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st, int
       	have_lock = 1;
       	err = stack_uptodate(st);
      @@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st, int first, int last,
     - 
     - 		sublock_file_fd = open(subtab_lock.buf,
     - 				       O_EXCL | O_CREAT | O_WRONLY, 0644);
     --		if (sublock_file_fd > 0) {
     -+		if (sublock_file_fd >= 0) {
     - 			close(sublock_file_fd);
     - 		} else if (sublock_file_fd < 0) {
     - 			if (errno == EEXIST) {
     -@@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st, int first, int last,
       		goto done;
       	}
       	err = close(lock_file_fd);
  7:  30016fee4c8 =  7:  afee67fb60d reftable: all xxx_free() functions accept NULL arguments
  8:  f90846cc103 !  8:  6f4f24c17ad reftable: order unittests by complexity
     @@ Metadata
       ## Commit message ##
          reftable: order unittests by complexity
      
     -    This is a more practical ordering when working on refactorings of the reftable
     -    code.
     +    This is a more practical ordering when working on refactorings of the
     +    reftable code.
      
          Signed-off-by: Han-Wen Nienhuys <[e-mail]>
      
  9:  425e12d3667 =  9:  0abaab1d7c4 reftable: drop stray printf in readwrite_test
 10:  67d858ec59e = 10:  004b7a736bf reftable: handle null refnames in reftable_ref_record_equal
 11:  1e50924894f = 11:  951efcebcd2 reftable: make reftable-record.h function signatures const correct
 12:  c81b17ad57e = 12:  6385e449ba7 reftable: implement record equality generically
 13:  4175089ec43 = 13:  69c4a554882 reftable: remove outdated file reftable.c
 14:  ede2e792ab6 ! 14:  54bcac325bd reftable: make reftable_record a tagged union
     @@ Metadata
       ## Commit message ##
          reftable: make reftable_record a tagged union
      
     -    This reduces the amount of glue code, because we don't need a void pointer or
     -    vtable within the structure.
     +    This reduces the amount of glue code, because we don't need a void
     +    pointer or vtable within the structure.
      
     -    The only snag is that reftable_index_record contain a strbuf, so it cannot be
     -    zero-initialized. To address this, use reftable_new_record() to return fresh
     -    instance, given a record type. Since reftable_new_record() doesn't cause heap
     -    allocation anymore, it should be balanced with reftable_record_release() rather
     -    than reftable_record_destroy().
     +    The only snag is that reftable_index_record contain a strbuf, so it
     +    cannot be zero-initialized. To address this, use reftable_new_record()
     +    to return fresh instance, given a record type. Since
     +    reftable_new_record() doesn't cause heap allocation anymore, it should
     +    be balanced with reftable_record_release() rather than
     +    reftable_record_destroy().
      
          Thanks to Peff for the suggestion.
      
 15:  6c06bd91662 = 15:  82f140cab5c reftable: add print functions to the record types
 16:  e16bf0c5212 <  -:  ----------- reftable: be more paranoid about 0-length memcpy calls