Karim Belabas on Thu, 17 Sep 2009 14:26:36 +0200


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

Re: Static analyzer run


* Bill Allombert [2009-09-15 22:41]:
> > ../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.

This is correct. It cannot return NULL when oE = 0 (and the last
argument is 0). Thus it *will* work the first time it is called.

The opa = NULL was necessary to silence an "uninitialized variable"
Warning...

Added a comment.

> > ../src/basemath/base3.c:1553:7: warning: Value stored to 'ex' is never read
> >       ex = EX;
> >       ^    ~~
> and
> > ../src/basemath/base3.c:1563:16: warning: Value stored to 'ex' is never read
> >         if (v) ex = mulii(ex, powiu(p, v));
> >                ^    ~~~~~~~~~~~~~~~~~~~~~~
> 
> Looking at the code, one wonder why this variable ex exists at all.
> Maybe the intent was 'EX = ex' instead, but that does not work.

The intent was to call famat_makecoprime with last argument 'ex', not 'EX'
( both are correct, the former divides the latter, making the call more
efficient ).

The whole "idealstar" structure (and all related code) is ridiculous in
any case : it stores mostly useless data and recomputes all the time the
actually needed quantities (such as this 'ex').  

Background : a few remaining code pieces still (wrongly) insist on
working with global algebraic integers instead of working locally modulo
suitable powers of primes; the idealstar structure was created when all
the code was entirely global. Have been working on cleaning it once and for 
all for some time now. Unfortunately, it in turn affects the bnr
structure, and changes all results in most regression tests, some of
them in annoying ways (much slower); never got to actually commit my
changes ...

> > ../src/basemath/bibli1.c:2129:14: warning: Value stored to 'j' is never read
> >         for (j = 0; i <= s; i++)
> >              ^   ~
> 
> Of course this is wrong, but what is the fix ???

No initialization needed:

         for (; i <= s; i++)

> > ../src/basemath/polarit3.c:2257:36: warning: Dereference of null pointer
> >   if (av2) { avma = av2; *lambda = next_lambda(*lambda); }
> >                                    ^

This is fine, we can never have lambda = NULL when av2 != 0. I made the
code less cryptic.

> > ../src/basemath/rootpol.c:2255:27: warning: Value stored to 'av2' is never read
> >       xd = RgX_deriv(xc); av2 = avma;
> >                           ^     ~~~~
> 
> It probably should be avma = av2 instead.

It should be deleted.

Thanks for looking into this !

Cheers,

    K.B.
--
Karim Belabas, IMB (UMR 5251)  Tel: (+33) (0)5 40 00 26 17
Universite Bordeaux 1          Fax: (+33) (0)5 40 00 69 50
351, cours de la Liberation    http://www.math.u-bordeaux1.fr/~belabas/
F-33405 Talence (France)       http://pari.math.u-bordeaux1.fr/  [PARI/GP]
`