Matematici:fidarsi, ma verificare

Matematici:fidarsi, ma verificare

A volte mi sento piuttosto imbarazzato quando esamino i bug nei progetti software. Molti di questi bug popolano il codice da molti anni e non puoi fare a meno di chiederti come il programma riesca ancora a funzionare con un centinaio di errori e difetti. E funziona in qualche modo. E le persone riescono a usarlo. Vale non solo per il codice che disegna un pockemon di un videogioco, ma anche per le librerie matematiche. La tua ipotesi è giusta:parleremo della libreria di matematica Scilab e dei suoi risultati di analisi in questo articolo.

Scilab

Oggi parleremo di frammenti di codice sospetti nel pacchetto matematico di Scilab. L'analisi è stata eseguita con lo strumento PVS-Studio.

Scilab è un pacchetto computazionale numerico multipiattaforma open source e un linguaggio di programmazione di alto livello a orientamento numerico. Può essere utilizzato per l'elaborazione del segnale, l'analisi statistica, il miglioramento delle immagini, le simulazioni di fluidodinamica, l'ottimizzazione numerica e la modellazione, la simulazione di sistemi dinamici espliciti e impliciti e (se è installato il corrispondente toolbox) le manipolazioni simboliche. Scilab è l'alternativa open source più completa a MATLAB. [Wikipedia].

Sito ufficiale:http://www.scilab.org/

Il sistema fornisce un gran numero di funzionalità:

Trame e animazioni 2D e 3D;
algebra lineare, matrici sparse;
funzioni polinomiali e razionali;
interpolazione, approssimazione;
simulazione:soluzione ODE e DE;
Scicos - un ibrido di un modellatore e simulatore di sistemi dinamici grafici;
ottimizzazioni differenziali e non differenziali;
elaborazione del segnale;
operazione simultanea;
statistiche;
un sistema di computer algebra;
Interfacce per Fortran, Tcl/Tk, C, C++, Java, LabVIEW.

Preparati:l'articolo sarà piuttosto lungo. Non sono io la colpa per così tanta sporcizia in questo codice e sono solo desideroso di mostrarti una gamma più ampia possibile di vari difetti.

I bug che ho trovato non hanno nulla a che fare con la matematica, ovviamente. Forse tutti gli algoritmi nella libreria sono corretti ed efficienti. Ma poiché gli sviluppatori hanno scelto di scrivere il loro programma in C++, avrebbero dovuto tenere a mente vari possibili problemi come errori di battitura, dereferenziazione del puntatore nullo e altri errori oltre agli errori negli algoritmi. Dopotutto, per un utente non fa alcuna differenza se si trova di fronte a un errore logico in un algoritmo numerico o se si tratta di una variabile non inizializzata che gli causerà molti problemi.

Certo, l'analisi statica può trovare solo determinati tipi di errori. Ma dal momento che sono facili da rilevare, perché non dovremmo farlo? È meglio correggere il 10% in più dei bug che non correggere nulla.

Vediamo quindi cosa ha da dirci il PVS-Studio sui bug nel progetto Scilab.

Un buffer inesistente

int sci_champ_G(....)
{
  ....
  char * strf = NULL ;
  ....
  if ( isDefStrf( strf ) )
  {
    char strfl[4];
    strcpy(strfl,DEFSTRFN);
    strf = strfl;
    if ( !isDefRect( rect ) )
    {
      strf[1]='5';
    }
  }

  (*func)(stk(l1), stk(l2), stk(l3), stk(l4),
    &m3, &n3, strf, rect, arfact, 4L);
  ....  
}

Messaggio diagnostico di PVS-Studio:V507 Il puntatore all'array locale 'strfl' è memorizzato al di fuori dell'ambito di questo array. Tale puntatore non sarà più valido. sci_champ.c 103

Un riferimento a un array temporaneo 'strfl' viene salvato nella variabile 'strf'. Quando si esce dal blocco "if () { ... }", questo array cessa di esistere. Tuttavia, il programmatore continua a lavorare con il puntatore 'strf'.

Questo porta a un comportamento indefinito. Non si può lavorare con un array già distrutto. Il programma potrebbe continuare a funzionare correttamente, ovviamente, ma solo per pura fortuna. L'area di memoria utilizzata per memorizzare quell'array può essere in qualsiasi momento occupata da altri array o variabili.

Frammenti con difetti simili:

Array 'strfl'. sci_fec.c 111
Array 'strfl'. sci_grayplot.c 94
Array 'strfl'. sci_matplot.c 84

Qualcosa di sbagliato calcolato

int C2F(pmatj)
  (char *fname, int *lw, int *j, unsigned long fname_len)
{
  ....
  ix1 = il2 + 4;
  m2 = Max(m, 1);
  ix1 = il + 9 + m * n;
  ....
}

Messaggio diagnostico di PVS-Studio:V519 Alla variabile 'ix1' vengono assegnati valori due volte di seguito. Forse questo è un errore. Righe di controllo:2387, 2389. stack1.c 2389

Qualcosa non va con la variabile 'ix1'. Immagino che ci sia un errore di battitura da qualche parte in questo codice.

Un controllo prima dell'inizializzazione

Questo è un pezzo di codice interessante:il programmatore aveva bisogno di ottenere alcuni valori e quindi controllarli. Ma tutto è andato proprio al contrario.

int sci_Playsound (char *fname,unsigned long fname_len)
{
  ....
  int m1 = 0, n1 = 0;
  ....
  if ( (m1 != n1) && (n1 != 1) ) 
  {
    Scierror(999,_("%s: Wrong size for input argument #%d: ")
                 _("A string expected.\n"),fname,1);
    return 0;
  }
  sciErr = getMatrixOfWideString(pvApiCtx, piAddressVarOne,
             &m1,&n1,&lenStVarOne, NULL);
  ....
}

Messaggi diagnostici di PVS-Studio:V560 Una parte dell'espressione condizionale è sempre falsa:(m1 !=n1). sci_playsound.c 66; V560 Una parte dell'espressione condizionale è sempre vera:(n1 !=1). sci_playsound.c 66

Le variabili m1 e n1 devono assumere valori quando si chiama la funzione getMatrixOfWideString() e quindi essere verificate. Tuttavia, il controllo sembra essere eseguito prima della chiamata della funzione.

Il controllo trova le variabili m1 e n1 uguali a 0, quindi la condizione "if ( (m1 !=n1) &&(n1 !=1) )" non sarà mai vera. Di conseguenza, questo controllo non influirà in alcun modo sull'esecuzione del programma.

Per riassumere, il controllo delle variabili m1 e n1 fallisce.

Numeri magici

void CreCommon(f,var)
     FILE *f;
     VARPTR var;
{
  ....
  if ( strncmp(var->fexternal, "cintf", 4)==0 )
  ....
}

Messaggio diagnostico di PVS-Studio:V666 Considerare di esaminare il terzo argomento della funzione 'strncmp'. È possibile che il valore non corrisponda alla lunghezza di una stringa passata con il secondo argomento. crerhs.c 119

Qui viene utilizzato il numero magico 4 e questo numero non è corretto. Ci sono 5 caratteri, non 4, nella stringa "cintf". Non usare questi numeri magici.

Avrei implementato una macro speciale per questo codice per calcolare la lunghezza delle stringhe letterali e usarla nel modo seguente:

if ( strncmp(var->fexternal, "cintf", litlen("cintf"))==0 )

Non discuteremo ora di come implementare la macro "litlen":ci sono molti modi per farlo per soddisfare i gusti di tutti. L'idea principale è sbarazzarsi del numero magico.

Altri frammenti con lunghezze di stringa errate:

crerhs.c 121
crerhs.c 123
crerhs.c 125
crerhs.c 127

1, 2, 3, 4, 4, 6

int C2F(run)(void)
{
  ....
  static int *Lpt = C2F(iop).lpt - 1;
  ....
  Lpt[1] = Lin[1 + k];
  Lpt[2] = Lin[2 + k];
  Lpt[3] = Lin[3 + k];
  Lpt[4] = Lin[4 + k];
  Lct[4] = Lin[6 + k ];
  Lpt[6] = k;
  ....
}

Messaggio diagnostico di PVS-Studio:V525 Il codice contenente la raccolta di blocchi simili. Controllare gli elementi '1', '2', '3', '4', '4' nelle righe 1005, 1006, 1007, 1008, 1009. run.c 1005

C'è un errore di battitura in una sequenza numerica. Risulta in un elemento dell'array che rimane non inizializzato. Questo può portarti molti risultati matematici interessanti.

Evoluzione del codice

int write_xml_states(
  int nvar, const char * xmlfile, char **ids, double *x)
{
  ....
  FILE *fd = NULL;
  ....
  wcfopen(fd, (char*)xmlfile, "wb");
  if (fd < 0)
  {
    sciprint(_("Error: cannot write to  '%s'  \n"), xmlfile);
    ....
}

Messaggio diagnostico di PVS-Studio:V503 Questo è un confronto senza senso:pointer <0. scicos.c 5826

Sono quasi sicuro che la funzione open sia stata utilizzata in passato in questo codice per aprire un file. Deve essere stata sostituita in seguito con la funzione _wfopen:la sua chiamata è nascosta all'interno della macro 'wcfopen'.

Tuttavia, il programmatore ha dimenticato di correggere il controllo della corretta apertura del file. La funzione open() restituisce il valore -1 in caso di errore e verificare che un puntatore sia inferiore a zero non ha alcun senso.

Ecco un altro frammento con lo stesso sfondo.

void taucs_ccs_genmmd(taucs_ccs_matrix* m,
  int** perm, int** invperm)
{
  int  n, maxint, delta, nofsub;
  ....
  maxint = 32000;
  assert(sizeof(int) == 4);
  maxint = 2147483647; /* 2**31-1, for 32-bit only! */
  ....
}

Messaggio diagnostico di PVS-Studio:V519 Alla variabile 'maxint' vengono assegnati valori due volte di seguito. Forse questo è un errore. Righe di controllo:154, 157. taucs_scilab.c 157

Non ci sono errori qui, ma il codice è piuttosto divertente.

Puoi notare la riga "maxint =32000;" scritto molto tempo fa. Quindi è stato aggiunto un nuovo pezzo di codice sotto di esso:

assert(sizeof(int) == 4);
maxint = 2147483647; /* 2**31-1, for 32-bit only! */

Ordinamento di un elemento

char *getCommonPart(char **dictionary, int sizeDictionary)
{
  ....
  char *currentstr = dictionary[0];
  qsort(dictionary, sizeof dictionary / sizeof dictionary[0],
        sizeof dictionary[0], cmp);
  ....
}

Messaggio diagnostico di PVS-Studio:V514 Dividere la dimensione di un puntatore 'sizeof dictionary' per un altro valore. C'è una probabilità di presenza di un errore logico. getcommonpart.c 76

Il secondo argomento della funzione qsort() è il numero di elementi in un array. A causa di un errore, questo numero ne fa sempre uno.

Dai un'occhiata all'espressione "sizeof dictionary / sizeof dictionary[0]":la dimensione del puntatore è divisa per la dimensione del puntatore. Questo restituisce uno.

Immagino che il codice corretto dovrebbe essere simile a questo:

qsort(dictionary, sizeDictionary, sizeof dictionary[0], cmp);

Un errore simile è stato riscontrato nel seguente frammento:getfilesdictionary.c 105

Stringhe ostinate

void GetenvB(char *name, char *env, int len)
{
  int ierr = 0, one = 1;
  C2F(getenvc)(&ierr,name,env,&len,&one);
  if (ierr == 0) 
  {
    char *last = &env[len-1];
    while ( *last == ' ' ) { last = '\0' ; } 
    last--;
  }
  ....
}

V527 È strano che il valore '\0' sia assegnato al puntatore di tipo 'char'. Probabilmente significava:*last ='\0'. getenvb.c 24

Questo codice è semplicemente orribile. O bello - se parliamo di errori dal punto di vista di quanto siano interessanti.

while ( *last == ' ' ) { last = '\0' ; }

Se il primo carattere nella stringa è uno spazio, il puntatore verrà impostato su zero e dopo ci occuperemo dell'accesso agli elementi su un puntatore nullo.

Sospetto che questo codice avesse lo scopo di sostituire tutti gli spazi con "\0". Se è così, dovrebbe apparire così:

while ( *last == ' ' ) { *last++ = '\0' ; }

È una cosa divertente, ma c'è un altro frammento nel codice in cui gli spazi devono essere sostituiti con zeri, e anche questo è fatto in modo errato.

static int msg_101(int *n, int *ierr)
{
  ....
  for (i=0;i<(int)strlen(line);i++)
  {
    if (line[i]==' ') line[i]='\0';
    break;
  }
  ....
}

Messaggio diagnostico di PVS-Studio:V612 Un'interruzione incondizionata all'interno di un ciclo. msgs.c 1293

Andrebbe tutto bene se non fosse per l'operatore 'break'. Verrà sostituito solo uno spazio. Tuttavia, rimuovere 'break' non aiuterà le cose:la funzione strlen() restituirà zero e il ciclo terminerà lo stesso.

Altri cicli "una tantum":

V612 Un'interruzione incondizionata all'interno di un ciclo. msgs.c 1313
V612 Un'interruzione incondizionata all'interno di un ciclo. api_common.cpp 1407

Dereferenziazione puntatore nullo

char **splitLineCSV(....)
{
  ....
  if (retstr[curr_str] == NULL)
  {
    *toks = 0;
    FREE(substitutedstring);
    substitutedstring = NULL;
    freeArrayOfString(retstr, strlen(substitutedstring));
    return NULL;
  }
  ....
}

Messaggio diagnostico di PVS-Studio:V575 Il puntatore nullo è passato alla funzione 'strlen'. Esamina il primo argomento. splitline.c 107

È un codice strano. Il programmatore prima azzera direttamente il puntatore 'substitutedstring' e poi lo lancia senza pietà come vittima della funzione strlen().

Sospetto che la chiamata della funzione freeArrayOfString() dovrebbe essere stata scritta prima della chiamata della funzione FREE().

Quello era un riscaldamento. Ora esaminiamo un caso più complesso.

inline static void create(void * pvApiCtx, const int position,
  const int rows, const int cols, long long * ptr)
{
  int * dataPtr = 0;
  alloc(pvApiCtx, position, rows, cols, dataPtr);
  for (int i = 0; i < rows * cols; i++)
  {
    dataPtr[i] = static_cast<int>(ptr[i]);
  }
}

V522 Potrebbe verificarsi un dereferenziamento del puntatore nullo 'dataPtr'. scilababstractmemoryallocator.hxx 222

Il programmatore voleva allocare memoria in questa funzione tramite alloc(). A prima vista può sembrare che la funzione restituisca un valore per riferimento, l'ultimo argomento rappresentato dal puntatore 'dataPtr' che serve a memorizzare il puntatore nel buffer di memoria allocato.

Ma è sbagliato. Il puntatore rimarrà nullo. Dai un'occhiata alla dichiarazione della funzione alloc():

inline static int *alloc(
  void * pvApiCtx, const int position, const int rows,
  const int cols, int * ptr)

Come puoi vedere, l'ultimo argomento non è un riferimento. A proposito, non è del tutto chiaro il motivo per cui è stato scritto qui. Diamo un'occhiata all'interno della funzione alloc():

inline static int *alloc(
  void * pvApiCtx, const int position, const int rows,
  const int cols, int * ptr)
{
  int * _ptr = 0;
  SciErr err = allocMatrixOfInteger32(
    pvApiCtx, position, rows, cols, &_ptr);
  checkError(err);
  return _ptr;
}

L'ultimo argomento 'ptr' non viene utilizzato affatto.

Ad ogni modo, il codice per l'allocazione della memoria è scritto in modo errato. Dovrebbe apparire come segue:

inline static void create(void * pvApiCtx, const int position,
  const int rows, const int cols, long long * ptr)
{
  int *dataPtr = alloc(pvApiCtx, position, rows, cols, 0);
  for (int i = 0; i < rows * cols; i++)
  {
    dataPtr[i] = static_cast<int>(ptr[i]);
  }
}

Altri problemi simili:

scilababstractmemoryallocator.hxx 237
scilababstractmemoryallocator.hxx 401

Messaggi di errore errati

PVS-Studio rivela parecchi errori di battitura nei gestori degli errori. Questi rami di codice vengono eseguiti raramente, quindi gli errori al loro interno potrebbero rimanere inosservati per molto tempo. Sospetto che sia a causa di questi errori che spesso non riusciamo a capire cosa c'è che non va nel programma:un messaggio di errore che genera non ha nulla a che fare con lo stato reale delle cose.

Ecco un esempio di errata preparazione del messaggio di errore:

static SciErr fillCommonSparseMatrixInList(....)
{
  ....
  addErrorMessage(&sciErr, API_ERROR_FILL_SPARSE_IN_LIST,
   _("%s: Unable to create list item #%d in Scilab memory"),
   _iComplex ? "createComplexSparseMatrixInList" :
               "createComplexSparseMatrixInList",
   _iItemPos + 1);
  ....
}

Messaggio diagnostico di PVS-Studio:V583 L'operatore '?:', indipendentemente dalla sua espressione condizionale, restituisce sempre lo stesso valore:"createComplexSparseMatrixInList". api_list.cpp 2398

Indipendentemente dal valore della variabile '_iComplex', il messaggio "createComplexSparseMatrixInList" verrà sempre stampato.

Altri problemi simili:

lista_api.cpp 2411
lista_api.cpp 2418
lista_api.cpp 2464
lista_api.cpp 2471

Ora discutiamo di un gestore di errori che non otterrà mai il controllo:

#define __GO_FIGURE__ 9
#define __GO_UIMENU__ 21
int sci_uimenu(char *fname, unsigned long fname_len)
{
  ....
  if (iParentType == __GO_FIGURE__ &&
      iParentType == __GO_UIMENU__)
  {
    Scierror(999, _("%s: Wrong type for input argument #%d: ")
             _("A '%s' or '%s' handle expected.\n"), 
             fname, 1, "Figure", "Uimenu");
    return FALSE;
  }
  ....
}

Il messaggio diagnostico di PVS-Studio:V547 Expression 'iParentType ==9 &&iParentType ==21' è sempre falso. Probabilmente il '||' operatore dovrebbe essere utilizzato qui. sci_uimenu.c 99

La condizione (iParentType ==__GO_FIGURE__ &&iParentType ==__GO_UIMENU__) non sarà mai vera. La variabile non può essere uguale a 9 e 21 contemporaneamente. Penso che il programmatore volesse che fosse scritto come segue:

if (iParentType != __GO_FIGURE__ &&
    iParentType != __GO_UIMENU__)

Un altro esempio, particolarmente gustoso.

int set_view_property(....)
{
  BOOL status = FALSE;
  ....
  status = setGraphicObjectProperty(
    pobjUID, __GO_VIEW__, &viewType, jni_int, 1);

  if (status = TRUE)
  {
    return SET_PROPERTY_SUCCEED;
  }
  else
  {
    Scierror(999, _("'%s' property does not exist ")
      _("for this handle.\n"), "view");
    return  SET_PROPERTY_ERROR ;
  }
  ....
}

Messaggio diagnostico di PVS-Studio:V559 Assegnazione sospetta all'interno dell'espressione della condizione dell'operatore 'if':status =1. set_view_property.c 61

L'errore è in questa riga:"if (status =TRUE)". L'assegnazione avviene invece del confronto.

Nessuna scelta rimasta

Questa funzione può ovviamente essere abbreviata. Deve essere stato scritto tramite il metodo Copy-Paste, il programmatore dimentica di modificare qualcosa nel testo copiato.

static int uf_union  (int* uf, int s, int t) {
  if (uf_find(uf,s) < uf_find(uf,t)) 
  {
    uf[uf_find(uf,s)] = uf_find(uf,t); 
    return (uf_find(uf,t)); 
  }
  else
  {
    uf[uf_find(uf,s)] = uf_find(uf,t); 
    return (uf_find(uf,t)); 
  }
}

Messaggio diagnostico di PVS-Studio:V523 L'istruzione 'then' è equivalente all'istruzione 'else'. taucs_scilab.c 700

Indipendentemente dalla condizione, viene eseguito lo stesso algoritmo.

Ecco un'altra situazione in cui abbiamo a che fare con condizioni coincidenti:

int sci_xset( char *fname, unsigned long fname_len )
{
  ....
  else if ( strcmp(cstk(l1), "mark size") == 0)
  ....
  else if ( strcmp(cstk(l1), "mark") == 0)  
  ....
  else if ( strcmp(cstk(l1), "mark") == 0)
  ....
  else if ( strcmp(cstk(l1), "colormap") == 0)
  ....
}

Messaggio diagnostico di PVS-Studio:V517 È stato rilevato l'uso del pattern 'if (A) {...} else if (A) {...}'. C'è una probabilità di presenza di un errore logico. Righe di controllo:175, 398. sci_xset.c 175

Qualche altra condizione errata:

sci_xset.c 159
h5_readdatafromfile_v1.c 1148
h5_readdatafromfile.c 1010

Classici

Sono sicuro che ora ho capito quale errore commettono più spesso i programmatori C/C++:prima dereferenziare un puntatore e solo dopo controllarlo per essere nullo. Non sempre causa un errore, ma non ci sono scuse per un codice così brutto.

static void appendData(....)
{
  ....
  sco_data *sco = (sco_data *) * (block->work);
  int maxNumberOfPoints = sco->internal.maxNumberOfPoints;
  int numberOfPoints = sco->internal.numberOfPoints;
  
  if (sco != NULL && numberOfPoints >= maxNumberOfPoints)
  ....
}

Messaggio diagnostico di PVS-Studio:V595 Il puntatore 'sco' è stato utilizzato prima di essere verificato rispetto a nullptr. Righe di controllo:305, 311. canimxy3d.c 305

Inizialmente, il programmatore si rivolgeva ai membri tramite il puntatore 'sco':

int maxNumberOfPoints = sco->internal.maxNumberOfPoints;
int numberOfPoints = sco->internal.numberOfPoints;

Poi gli venne in mente che il puntatore avrebbe dovuto essere controllato:

if (sco != NULL .....

L'analizzatore ha generato altri 61 avvisi V595. Non trovo ragionevole enumerarli tutti nell'articolo, quindi ecco un elenco separato:scilab-v595.txt.

Un errore più diffuso è quello di utilizzare identificatori di formato errati quando si lavora con la funzione sprint() e simili. Non c'erano quasi esempi interessanti tra tutti i problemi di questo tipo:sono solo valori non firmati stampati come quelli firmati. Ecco perché ho creato un altro elenco:scilab-v576.txt.

L'unico campione interessante che potrei individuare è il seguente:

#define FORMAT_SESSION "%s%s%s"
char *getCommentDateSession(BOOL longFormat)
{
  ....
  sprintf(line, FORMAT_SESSION, SESSION_PRAGMA_BEGIN,
          STRING_BEGIN_SESSION, time_str, SESSION_PRAGMA_END);
  ....
}

Messaggio diagnostico di PVS-Studio:V576 Formato errato. È previsto un numero diverso di argomenti effettivi durante la chiamata alla funzione 'sprintf'. Previsto:5. Presente:6. getcommentdatesession.c 68

La stringa SESSION_PRAGMA_END non verrà stampata.

Attenzione! Comportamento indefinito!

short ezxml_internal_dtd(ezxml_root_t root, char *s, size_t len)
{
  ....
  while (*(n = ++s + strspn(s, EZXML_WS)) && *n != '>') {
  ....
}

Messaggio diagnostico di PVS-Studio:V567 Comportamento non definito. La variabile 's' viene modificata mentre viene utilizzata due volte tra i punti della sequenza. ezxml.c 385

Non si può dire con certezza quale delle due espressioni "++s' o "strspn(s, EZXML_WS)" verrà calcolata per prima. Pertanto, potresti ottenere risultati diversi con diversi compilatori, piattaforme, ecc.

Ecco un altro esempio, più interessante. In questo codice, un errore di battitura porta a un comportamento indefinito.

static char **replaceStrings(....)
{
  ....
  int i = 0;
  ....
  for (i = 0; i < nr; i = i++)
  ....
}

V567 Comportamento non definito. La variabile 'i' viene modificata mentre viene utilizzata due volte tra i punti della sequenza. csvread.c 620

Il problema è in questo pezzo:i =i++.

Molto probabilmente il codice doveva essere scritto in questo modo:

for (i = 0; i < nr; i++)

Qualche parola in più sulle stringhe

char *PLD_strtok(....)
{
  ....
  if ((st->start)&&(st->start != '\0'))
  ....
}

Messaggio diagnostico di PVS-Studio:V528 È strano che il puntatore al tipo 'char' venga confrontato con il valore '\0'. Probabilmente significava:*st->start !='\0'. pldstr.c 303

Il programmatore voleva controllare che la stringa non fosse vuota, ma in realtà ha confrontato il puntatore con NULL due volte. Il codice fisso dovrebbe apparire come segue:

if ((st->start)&&(st->start[0] != '\0'))

Un altro errore di questo tipo:

V528 È strano che il puntatore al tipo 'char' venga confrontato con il valore '\0'. Probabilmente significava:** categoria =='\0'. sci_xcospalload.cpp 57

Il frammento di codice seguente sembra essere incompleto:

int sci_displaytree(char *fname, unsigned long fname_len)
{
  ....
  string szCurLevel = "";
  ....
  //Add node level
  if (szCurLevel != "")
  {
    szCurLevel + ".";
  }
  ....
}

Avviso di PVS-Studio:V655 Le stringhe sono state concatenate ma non vengono utilizzate. Prendi in considerazione l'esame dell'espressione 'szCurLevel + "."'. sci_displaytree.cpp 80

Codice che funziona per pura fortuna

static int sci_toprint_two_rhs(void* _pvCtx,
                               const char *fname)
{
  ....
  sprintf(lines, "%s%s\n", lines, pStVarOne[i]);
  ....
}

Avvertimento di PVS-Studio:V541 È pericoloso stampare le 'linee' della stringa su se stessa. sci_toprint.cpp 314

La funzione sprintf() salva il risultato di ritorno nel buffer 'lines'. Allo stesso tempo, questo stesso buffer è anche una delle stringhe di input. Non va bene fare queste cose. Il codice potrebbe funzionare, ma è molto rischioso. Se passi a un altro compilatore, potresti ottenere un risultato inaspettato e molto spiacevole.

Un altro difetto del genere:sci_coserror.c 94

Ecco un esempio di codice errato che funziona bene:

typedef struct JavaVMOption {
    char *optionString;
    void *extraInfo;
} JavaVMOption;

JavaVMOption *options;

BOOL startJVM(char *SCI_PATH)
{
  ....
  fprintf(stderr, "%d: %s\n", j, vm_args.options[j]);
  ....
}

Messaggio diagnostico di PVS-Studio:V510 La funzione 'fprintf' non dovrebbe ricevere la variabile di tipo classe come quarto argomento effettivo. jvm.c 247

Il programmatore voleva stampare la stringa a cui si riferisce il puntatore 'optionString'. Il codice corretto dovrebbe assomigliare a questo:

fprintf(stderr, "%d: %s\n", j, vm_args.options[j].optionString);

Tuttavia, la funzione fprintf() prenderà effettivamente un oggetto del tipo JavaVMOption come argomento. Il codice funziona solo grazie a una meravigliosa e fortunata coincidenza.

Innanzitutto, il membro 'optionString' si trova all'inizio della struttura. Ecco perché è questo particolare membro che la funzione fprintf() prenderà e gestirà come un puntatore alla stringa.

In secondo luogo, la funzione non stamperà nulla dopo, quindi non verrà stampata nemmeno la spazzatura (cioè il contenuto della variabile 'extraInfo' che entrerà anche nello stack).

Alleluia!

Ciclo difettoso

static void reinitdoit(double *told)
{
  int keve = 0, kiwa = 0;
  ....
  kiwa = 0;
  ....
  for (i = 0; i < kiwa; i++)
  ....
}

V621 Considerare di ispezionare l'operatore 'for'. È possibile che il ciclo venga eseguito in modo errato o non venga eseguito affatto. scicos.c 4432

Qualcosa non va, qui. La variabile 'kiwa' è sempre uguale a zero. Il ciclo non itera. Forse questo codice è incompleto.

Cosa non è stato incluso nell'articolo

Ad essere onesto, sono troppo stanco per scansionare il rapporto e scrivere questo articolo. Quindi è meglio che mi fermi qui. Potrei citare qualche frammento più sospetto, ma non li ho trovati troppo significativi e ho ceduto alla pigrizia. Inoltre, sono sicuro di essermi perso qualcosa, perché non ho familiarità con il progetto. A questo proposito, raccomando agli autori del progetto di verificare autonomamente il loro codice con l'analizzatore PVS-Studio.

Nota. Voglio ricordare a coloro che credono di poter utilizzare l'analizzatore di codice statico per un controllo una tantum senza acquistarlo che non avrà alcun senso. Ciò di cui tratta l'analisi statica sono controlli regolari, non una tantum. Quando si commette un errore di battitura, l'analizzatore lo rileva immediatamente, riducendo così il tempo impiegato per testare, eseguire il debug e correggere i bug dal bug tracker. Per saperne di più, consulta l'articolo "Leo Tolstoj e l'analisi statica del codice".

Nota

Qualcuno sicuramente chiederà quale versione di Scilab stavo controllando. Sfortunatamente, non era il più fresco. Ho controllato questo progetto e annotato i frammenti di codice sospetti circa un mese e mezzo fa... e me ne sono completamente dimenticato perché eravamo molto impegnati con un confronto di analizzatori per quel tempo. E proprio di recente mi sono imbattuto in quel file e mi ci è voluto un bel po' per ricordare con cosa ha a che fare. Vedi, devo controllare così tanti progetti che sono tutti confusi nella mia testa e non riesco nemmeno a ricordare se ho già controllato questo o quel progetto.

Bene, va bene comunque. Ora finirò con questo articolo e gli autori di Scilabe lo noteranno e verificheranno loro stessi il loro progetto. L'obiettivo principale dei miei articoli è mostrare le capacità della metodologia di analisi statica, non trovare ogni singolo bug nelle ultime versioni del progetto.

Conclusione

Assicurati di utilizzare regolarmente l'analisi statica:solo allora ti aiuterà a risparmiare tempo per correggere errori stupidi per dedicarlo invece a qualcosa di più utile.

Progetti di medie e grandi dimensioni che non possono fare a meno di controlli notturni, personalizzazione degli strumenti aggiuntivi, integrazione con MSBuild, supporto di Visual Studio 2005/2008 e così via, sono invitati a provare l'analizzatore PVS-Studio.

Riferimenti

Terminologia. Analisi del codice statico.
Andrey Karpov, Evgeniy Ryzhkov, Paul Eremeev, Svyatoslav Razmyslov. Confronto di analizzatori di codice statico:CppCat, Cppcheck, PVS-Studio e Visual Studio. (metodologia di confronto).