On Tue, Dec 07, 2021 at 12:34:15PM +0100, Han-Wen Nienhuys wrote: > > - A lot of your structs have vtables. Initializing them to NULL, as in > > reftable_reader_refs_for_indexed(), leaves the risk that we'll try > > to call a NULL function pointer, even if it's for something simple > > I have the impression that coverity doesn't understand enough of the > control flow. Some of the things it complains of are code paths that > only get executed if err==0, in which case, the struct members at hand > should not be null. I've definitely run into cases where it doesn't understand some invariant (e.g., "foo" is only nonzero if "bar" is not NULL). But the ones I looked at here were triggerable. It's a lot easier to see via their site which details there view of the control flow... > > 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 similarly wondered if these polymorphic types could be using a union > > within reftable_record, rather than pointing to a separate stack > > variable. Then you could initialize the whole thing without worrying > > about intermediate NULLs (and also there's less pointer chasing and it's > > a little bit more type safe than a void pointer). But again, I don't > > know the code well enough to know if that would cover all of your cases. > > This is a great idea. I've made a change that does this, which I will > post shortly. Oh good. I was worried I was going off on a tangent. I'll give your patches a look. -Peff
