Ist das eine schlechte Programmierpraxis?

Question

Ich verwende PC-Lint, um meinen Code zu analysieren, und diese Zeilen erzeugen mehrere Fehler. Das lässt mich fragen, ob meine Codierungspraxis falsch ist?

char *start; 
char *end; 

// Extract the phone number 
start = (char*) (strchr(data, '\"') +1); 
end = (char*) strchr(start, '\"'); 
*end = 0; 
strlcpy((char*)Fp_smsSender, start , start-(end-1)); 

EDIT: Nach Ihrer Hilfe, ich habe jetzt:

char *start; 
char *end; 

if (data != NULL) 
{ 
    // Extract the phone number 
    start = strchr(data, '\"'); 
    if (start != NULL) 
    { 
    ++start; 
    end = strchr(start, '\"'); 

    if (end != NULL) 
    { 
     *end = 0; 
     strlcpy((char*)Fp_smsSender, start , FP_MAX_PHONE); 
    } 
    } 

Wie das aus?

Answer 1

Ich stelle mir vor, dass das, worüber lint sich beschwert, ist, dass strchr() einen NULL-Zeiger zurückgeben kann, und Sie nicht nach dem vor der Ausführung von Zeigerarithmetik und Dereferenzierung es überprüfen.

Sie könnte so etwas wie zu tun:

char *start; 
char *end; 

// Extract the phone number 
start = strchr(data, '\"'); 
if (!start) handle_error(); 

++start; // skip the '\"' 
end = strchr(start, '\"'); 
if (!end) handle_error(); 

*end = 0; 
strlcpy((char*)Fp_smsSender, start, size_of_Fp_smsSender_buffer); 

Bitte beachte, dass ich den letzten Parameter auf den strlcpy() Aufruf geändert - was dieser Parameter für ist die Größe des Zielpuffers zu spezifizieren, so dass Sie nicht tun überrannt es. Der Wert, den du passierst, ergibt überhaupt keinen Sinn, und Lint hat sich wahrscheinlich auch darüber beschwert. Sie haben wahrscheinlich end-(start-1) gemeint, was einfacher als strlen(start)+1 angegeben werden kann.

Wie auch immer, auch in strlen(start)+1 als letzten Parameter zu strlcpy() übergeben verletzt die Absicht des Parameters, und entfernt die Sicherheit strlcpy() soll liefern. Sie könnten auch einfach strcpy(Fp_smsSender,start) verwendet haben - und wenn Sie nicht wissen, wie groß der Fp_smsSender Zielpuffer ist, sollten Sie genau das tun (oder Dinge reparieren, damit Sie wissen, wie groß der Puffer ist). Es wird klarer, was der Code tatsächlich macht.

Answer 2

Zwei Dinge: Erstens behandeln Sie keine NULL-Returns von strchr.

Sekunde (und ernster), die Länge Sie strlcpy passieren, ist falsch: Sie end - start oder etwas Ähnliches wollen würde (Sie haben das umgekehrt), aber im Grunde ist die Länge Argument strlcpy sollte die Größe des sein Ziel Puffer, nicht die Quellzeichenfolge.