Re: [PATCH] Fix buffer not null terminated on (ecpg lib)

At Thu, 23 Apr 2020 01:21:21 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote in
> Em qua., 22 de abr. de 2020 às 23:27, Kyotaro Horiguchi <
> horikyota(dot)ntt(at)gmail(dot)com> escreveu:
> >
> > - strncpy(sqlca->sqlerrm.sqlerrmc, message,
> > sizeof(sqlca->sqlerrm.sqlerrmc));
> > - sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1] = 0;
> > + sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1] =
> > '\0';
> > + strncpy(sqlca->sqlerrm.sqlerrmc, message,
> > sizeof(sqlca->sqlerrm.sqlerrmc) - 1);
> >
> > The existing strncpy then terminating by NUL works fine. I don't think
> > there's any point in doing the reverse way. Actually
> > sizeof(sqlca->sqlerrm.sqlerrmc) - 1 is enough for the length but the
> > existing code is not necessarily a bug.
> >
> Without understanding then, why Coveriy claims bug here.

Well, handling non-terminated strings with str* functions are a sign
of bug in most cases. Coverity is very useful but false positives are
annoying. I wonder what if we attach Coverity annotations to such
codes.

By the way, do you have some ideas of how to let coverity detect
leakage of resources other than memory? We found several cases of
cache reference leakage that should be statically detected easily.

https://www.postgresql.org/message-id/10513.1587143235@sss.pgh.pa.us
> I wonder whether there is any way to teach Coverity, or some other
> static analyzer, to look for code paths that leak cache refcounts.
> It seems isomorphic to detecting memory leaks, which Coverity is
> reasonably good at.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center