2017-10-12 15 views
1

Ich versuche, zwei Strings zu verketten, damit ich einen Dateipfad erhalten kann. Allerdings bin ich in valgrindC Strcat Valgrind Fehler

Bedingter Sprung oder Bewegung einen Fehler empfängt, hängt von uninitialised Wert (e)

Mein Code:

/** 
* @brief Concatenate two strings to get file path 
* @param firstP - First string 
* @param secondP - Second string 
* @return Returns the concatenated string 
*/ 
char *getPathDir(char *firstP, char *secondP) { 
    char *new_str; 
    int stringSize = strlen(firstP)+strlen(secondP)+2; 

    if((new_str = malloc(stringSize)) != NULL){ 
     new_str[0] = '\0'; 
     strcat(new_str,firstP); 
     new_str[strlen(firstP)] = '/'; 
     strcat(new_str,secondP); 
    } else { 
     perror("malloc"); 
     cleanUp(); 
     exit(EXIT_FAILURE); 
    } 
    return new_str; 
} 
+5

'sprintf (new_str,"% s /% s ", firstP, secondP);' statt. 'new_str [strlen (firstP)] = '/';' überschreibt den letzten Null-Terminator der Zeichenkette. So kann das zweite 'strcat' das Ende der korrekten Zeichenfolge nicht finden. – BLUEPIXY

+1

Es gibt viel bessere Möglichkeit, dies zu tun ... strcat-Funktion. Siehe: https://www.tutorialspoint.com/c_standard_library/c_function_strcat.htm oder @BLUEPIXY Vorschlag auch :) –

+0

@BLUEPIXY Ich habe alles innerhalb der if-Anweisung entfernt und ersetzte es mit sprintf (new_str, "% s /% s" , firstP, secondP); es funktioniert jetzt perfekt. Danke :) – Cows42

Antwort

6

Lassen Sie uns diese Zeilen aussehen:

new_str[0] = '\0'; 
    strcat(new_str,firstP); 
    new_str[strlen(firstP)] = '/'; 
    strcat(new_str,secondP); 

Bevor Sie etwas schreiben, sieht die Zeichenfolge wie folgt aus:

 +---+---+---+---+---+---+---+---+ 
    | ? | ? | ? | ? | ? | ? | ? | ? | 
    +---+---+---+---+---+---+---+---+ 

nach der ersten Zeile (new_str[0] = '\0';), haben Sie dies:

 +---+---+---+---+---+---+---+---+ 
    | 0 | ? | ? | ? | ? | ? | ? | ? | 
    +---+---+---+---+---+---+---+---+ 

Nach der zweiten Zeile (strcat(new_str,firstP);), es sieht wie folgt aus:

 +---+---+---+---+---+---+---+---+ 
    | A | B | C | D | 0 | ? | ? | ? | 
    +---+---+---+---+---+---+---+---+ 

Wenn Sie jetzt Führen Sie die Zeile

new_str[strlen(firstP)] = '/'; 

Sie den Nullabschlussüberschreiben und erhalten diese:

 +---+---+---+---+---+---+---+---+ 
    | A | B | C | D |/| ? | ? | ? | 
    +---+---+---+---+---+---+---+---+ 

Dies ist ein Problem, weil die Zeichenfolge nicht mehr nullterminierte ist, also wenn Sie strcat nächsten Anruf wird das Programm liest in nicht initialisierten Speicher Jagd beginnen um für ein Null-Terminator

Wenn Sie die Saiten zusammen verketten möchten, könnte es einfacher sein, nur sprintf zu verwenden, wie folgt aus:

sprintf(new_str, "%s/%s", firstP, secondP); 

Dieses mehr sagt ausdrücklich „den ersten String schreiben, dann das Trennzeichen, dann die zweite Zeichenfolge "und entlädt die gesamte Null-Terminator-Verwaltung in die Bibliothek. Und Bibliotheken, mit Ausnahme von strncat, behandeln normalerweise Null-Terminatoren ziemlich gut. :-)

Es gibt auch eine Chance, dass die sprintf marginal schneller als das, was Sie tun, ist. Mit vielen strcat s in der Art, wie Sie vorschlagen, kann aufgrund der overhead of rescanning the strings to find the null terminators ineffizient sein, aber ich würde nicht darauf wetten. Es hat jedoch den sehr klaren Vorteil, dass es genauer kommuniziert, was es zu tun versucht, und Lesbarkeitsgewinne sind selten eine schlechte Sache.

+0

Ausgezeichnete Möglichkeit, das Problem zu präsentieren. – Barmar

+0

Aber Sie könnten auch eine einfache Lösung zeigen, die die notwendige Null hinzufügt, oder verwendet 'strcat (new_str,"/")' – Barmar

+0

@Barmar Das stimmt, aber aus Gründen der Lesbarkeit denke ich, das 'sprintf' ist eine bessere Option.Es gibt auch die (sehr vernachlässigbare) ineffiziente Wiederabtastung von Strings, die immer wieder entsteht, wenn man viele "strcat's hintereinander verwendet. – templatetypedef