TL; DR : non si tratta di un problema di scoping dei blocchi, ma piuttosto di un malinteso su come devono essere gestiti i puntatori.
Per gentile concessione della scheda di vulnerabilità che hai pubblicato, diamo un'occhiata al codice in questione.
489 int
490 OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
491 {
...
494 char *bndec = NULL;
...
516 len = a->length;
...
519 while (len > 0) {
...
570 bndec = BN_bn2dec(bl);
571 if (!bndec)
572 goto err;
573 i = snprintf(buf, buf_len, ".%s", bndec);
...
598 }
...
601 free(bndec);
...
609 }
E se approfondiamo la dichiarazione di BN_bn2dec
vediamo un commento di:
/* Must 'free' the returned data */
Quello che vedi come un problema di scoping del blocco, vedo una scarsa comprensione delle operazioni del puntatore come problema di root.
Questo revisione diff mostra quando *bndec
è stata spostata dall'interno del ciclo while all'esterno del ciclo while. L'errore è stato anche introdotto nello stesso momento in cui l'istruzione free(bndec)
è stata spostata all'esterno del ciclo.
Dove *bndec
è dichiarato per questa funzione è un po 'irrilevante. Può essere tranquillamente definito all'esterno o all'interno del ciclo while. E se la memoria funziona correttamente, la dichiarazione effettiva verrà comunque spostata in cima alla funzione dal compilatore.
Dichiarare *bndec
all'interno del ciclo semplicemente avrebbe forzato un errore se la chiamata free()
rimanesse fuori dal ciclo. Resta da vedere se quell'errore sarebbe stato sufficiente per avvisare il programmatore della fallacia dietro la logica del refactoring.
Essendo un po 'pessimista quando si tratta della maggior parte dei programmatori e dei puntatori, ho intenzione di dire che il free
che accede a un errore di variabile fuori portata non li avrebbe aiutati a scoprire l'errore dei loro modi . Invece, avrebbero preso la soluzione ingenua (e non corretta) di risolvere l'errore semplicemente spostando la dichiarazione della variabile all'esterno del blocco. Ed è per questo che non sono d'accordo con la tesi che la causa principale di questo è un problema di scoping dei blocchi.
Se C avesse la garbage collection, allora il scope scope avrebbe avuto più valore in questo caso. Ma non è così, e quella linea di pensiero diventa una speculazione inutile.
Sfortunatamente, non sappiamo veramente perché il programmatore di refactoring ha scelto di spostare quell'istruzione free(bndec)
. Tutto ciò che hanno lasciato tramite i commenti del check-in è stato questo:
Clean up some of the nightmare of string and pointer arithmatic in
this nasty function.
This gets rid of the nasty tmp variables used to hold temporary strings
and the DECIMAL_SIZE hack. it gets rid of the rather pointless null checks
for buf (since the original code dereferences it before checking). It also
gets rid of the insane possibility this could return -1 when stuff is
using the return values to compute lengths All the failure cases now
return 0 and an empty string like the first error case in the original
code.
E mentre rispetto la loro dedizione al codice pulito, non sono certo di essere pienamente d'accordo con la rimozione di vari controlli dal codice. Avendo scritto una discreta quantità di codice C con una squadra abbastanza grande di sviluppatori, la mia esperienza è che le pratiche di codifica difensive sono quasi sempre garantite. E mentre uno sviluppatore potrebbe non essere in grado di giustificare le spese per aggiungere quelle pratiche difensive, non posso dire di aver mai trovato motivi validi per rimuovere quelle difese.
Per addentrarmi in un'uguale quantità di iperbole come programmatore di refactoring, trovo il loro commento di check-in per schiacciare un po 'troppo di hybris e non abbastanza di umiltà. È probabile che l'hubris li abbia portati ad assumere che "sapevano cosa stavano facendo" (TM) nel trasferire l'enunciazione free(bndec)
, e non hanno mentalmente camminato su quello che avrebbe fatto per il codice.
Questo errore è stato introdotto con il refactoring non a causa dello scope scope, ma a causa dell'incapacità di comprendere come funzionano i puntatori e di non capire come i puntatori si riferiscono alla memoria.
Anche se penso di aver stabilito perché il blocco degli ambiti sia irrilevante per questo problema, voglio indirizzare ciò che vedi come "great resistance to block scoped variables within old-school projects"
. E per quanto ne so, ANSI C ha sempre avuto a disposizione scope per le variabili. Uscendo su un arto, sospetto che K & C lo avesse anche.
Ricorda che C non è un linguaggio gentile per l'ingenuo o l'iniziato. È stato creato per fornire un ulteriore livello di astrazione su B
e programmazione a livello di assieme. Ci si aspettava che i programmatori che scrivono in C avessero una comprensione dell'assemblatore sottostante che veniva generato. Ad esempio, le variabili non vengono inizializzate automaticamente e molti programmatori sono stati masterizzati da tale mancanza di inizializzazione. C si aspetta che il programmatore capisca a cosa la dichiarazione delle variabili sia associata in memoria. C ha anche più della sua giusta dose di regole di stile non scritte che sono considerate "buone forme".
Lo spostamento di tutte le dichiarazioni delle variabili è una di quelle regole di stile non scritte e la ragione pragmatica risale alla mancanza di inizializzazione automatica. I programmatori C disciplinati hanno dichiarato tutte le loro variabili all'inizio della funzione in modo da poter eseguire una scansione visiva rapida per verificare che tutte le variabili dichiarate siano state inizializzate. È un modo semplice per assicurarsi che sia stato evitato un errore comune (ad esempio, la mancanza di inizializzazione).