Lorenz Minder on Wed, 16 Sep 2009 06:07:34 +0200


[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

Re: Static analyzer run


Hi,

BA:
> On Mon, Sep 14, 2009 at 10:00:32AM +0200, Lorenz Minder wrote:
> > It found a couple of minor things, such as duplicate assignments.  I
> > think the benefit of fixing those is mostly code readability.  Those
> > are in the attached minor_bugs.patch.
> 
> I have committed your patch with very small changes.

Thanks for looking into those.  FWIW, I agree that possibly not all of
them were desirable.  I'd in fact already weeded out quite a few changes
which I thought didn't improve matters before sending in, but there
may have been some more.

> > Then it uncovered a few real bugs. I (hope I) fixed those in
> > sa_bugfixes.patch.
> 
> Also done.

Great.

> Can you tell the analyzer that pari_err does not return ?
> This is a large source of false positive with gcc.

Hmm, isn't it gcc-clean at the moment?   In any case, maybe we should
declare pari_err() with __attribute__((noreturn)).  That would
presumably help both gcc and clang (whose analyzer I used to find these
problems).

I've had no time yet to look into how to make such a change in
a portable manner.

FWIW, I've indeed had a large number of false positives with clang due
to this.

> > ../src/basemath/base2.c:1637:16: warning: Dereference of null pointer
> >       S->phi = typ(opa) == t_INT? RgX_Rg_add_shallow(S->phi, opa)
> >                ^
> 
> I do not understand why it work, but clearly the statement
> opa = NULL; 
> is useless. Somehow the code assume that getprime will work the first
> time.
[...]

Ok, thanks for those assessments.  I hope I'll get around looking at
some of the ones that probably aren't false at some point. (Most of
them are in code I don't understand and therefore won't touch, though.)

BTW, I accidentally stumbled over a problem in the SEA module today.
Can you check the attached patch?

Best,
--Lorenz
-- 
GRATIS für alle GMX-Mitglieder: Die maxdome Movie-FLAT!
Jetzt freischalten unter http://portal.gmx.net/de/go/maxdome01

Attachment: ellsea_gerepileall.patch
Description: Binary data