Re: Fwd: coverity problems in reftable code

[Jeff King <peff@peff.net>]

On Tue, Dec 07, 2021 at 08:46:12PM -0500, Jeff King wrote:

> > > The summary of issues is below. You can get more details on their site.
> > > I _think_ I've configured it so that anybody can look at:
> > >
> > >   https://scan.coverity.com/projects/peff-git/view_defects
> > 
> > Alas, it says I have no access, even after I logged in.
> 
> ...hrmph. I have it "open to all users", but maybe you have to be
> associated with the project. I'll send you an "invite" through the
> Coverity site and see if that works (of course don't feel obligated if
> you don't want to deal further with Coverity; it _does_ produce a ton of
> false positives, but it sometimes says useful things, too).

I also applied your coverity fixups to my tree, and pushed up the
result. As expected, Coverity claims many problems fixed, but also a few
new ones found.

Summary is below, but I'm not sure it's that useful without the whole
code flow. The unreachable-code one seems just wrong. We can get there
via the "goto done" in the BLOCK_TYPE_LOG conditional, it looks like.

The first FORWARD_NULL doesn't look obvious to me from the code. But it
triggers a segfault running "test-tool reftable". (It didn't immediately
for me on Linux, but Windows CI shows it, and compiling with ASan on
Linux does too).

-- >8 --
** CID 1467061:  Null pointer dereferences  (FORWARD_NULL)
/reftable/record_test.c: 356 in test_reftable_obj_record_roundtrip()

________________________________________________________________________________________________________
*** CID 1467061:  Null pointer dereferences  (FORWARD_NULL)
/reftable/record_test.c: 356 in test_reftable_obj_record_roundtrip()
350     
351     		EXPECT(in.u.obj.hash_prefix_len == out.u.obj.hash_prefix_len);
352     		EXPECT(in.u.obj.offset_len == out.u.obj.offset_len);
353     
354     		EXPECT(!memcmp(in.u.obj.hash_prefix, out.u.obj.hash_prefix,
355     			       in.u.obj.hash_prefix_len));
>>>     CID 1467061:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "out.u.obj.offsets" to "memcmp", which dereferences it.
356     		EXPECT(0 == memcmp(in.u.obj.offsets, out.u.obj.offsets,
357     				   sizeof(uint64_t) * in.u.obj.offset_len));
358     		strbuf_release(&key);
359     		reftable_record_release(&out);
360     	}
361     }

** CID 1467060:  Possible Control flow issues  (DEADCODE)
/reftable/block.c: 263 in block_reader_init()


________________________________________________________________________________________________________
*** CID 1467060:  Possible Control flow issues  (DEADCODE)
/reftable/block.c: 263 in block_reader_init()
257     	br->header_off = header_off;
258     	br->restart_count = restart_count;
259     	br->restart_bytes = restart_bytes;
260     
261     done:
262     	if (uncompressed) {
>>>     CID 1467060:  Possible Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "reftable_free(uncompressed);".
263     		reftable_free(uncompressed);
264     	}
265     	return err;
266     }
267     
268     static uint32_t block_reader_restart_offset(struct block_reader *br, int i)

** CID 1467059:  Null pointer dereferences  (FORWARD_NULL)


________________________________________________________________________________________________________
*** CID 1467059:  Null pointer dereferences  (FORWARD_NULL)
/reftable/record_test.c: 155 in test_reftable_ref_record_roundtrip()
149     		EXPECT(n > 0);
150     
151     		/* decode into a non-zero reftable_record to test for leaks. */
152     		m = reftable_record_decode(&out, key, i, dest, GIT_SHA1_RAWSZ);
153     		EXPECT(n == m);
154     
>>>     CID 1467059:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing "&out.u.ref" to "reftable_ref_record_equal", which dereferences null "out.u.ref.refname".
155     		EXPECT(reftable_ref_record_equal(&in.u.ref, &out.u.ref,
156     						 GIT_SHA1_RAWSZ));
157     		reftable_record_release(&in);
158     
159     		strbuf_release(&key);
160     		reftable_record_release(&out);

** CID 1467058:  Null pointer dereferences  (FORWARD_NULL)
/reftable/record_test.c: 354 in test_reftable_obj_record_roundtrip()


________________________________________________________________________________________________________
*** CID 1467058:  Null pointer dereferences  (FORWARD_NULL)
/reftable/record_test.c: 354 in test_reftable_obj_record_roundtrip()
348     					   GIT_SHA1_RAWSZ);
349     		EXPECT(n == m);
350     
351     		EXPECT(in.u.obj.hash_prefix_len == out.u.obj.hash_prefix_len);
352     		EXPECT(in.u.obj.offset_len == out.u.obj.offset_len);
353     
>>>     CID 1467058:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "out.u.obj.hash_prefix" to "memcmp", which dereferences it.
354     		EXPECT(!memcmp(in.u.obj.hash_prefix, out.u.obj.hash_prefix,
355     			       in.u.obj.hash_prefix_len));
356     		EXPECT(0 == memcmp(in.u.obj.offsets, out.u.obj.offsets,
357     				   sizeof(uint64_t) * in.u.obj.offset_len));
358     		strbuf_release(&key);
359     		reftable_record_release(&out);