2016-04-04 15 views
0

Ich habe Probleme zu identifizieren, warum es einen Segmentierungsfehler in meinem Programm gibt. Ich habe solche Dinge schon einmal erfolgreich gemacht, aber diese spezielle Funktion verursacht einen Segmentierungsfehler.Segmentierungsfehler - sprintf

Mein Code ist hier:

void logErrorStatus(char* message) 
{ 
    char* errorMessage = "test"; 

    sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

    logMessage(errorMessage); 
} 


void logMessage(char* message) 
{ 
    FILE* file = NULL; 

    char buffer[SIZE]; 
    struct tm *sTm; 

    time_t now = time(NULL); 
    sTm = localtime(&now); 

    strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", sTm); 

    file = fopen(LOGFILE, "a"); 

    if(file == NULL) 
    { 
     printf("Error opening log file.\n"); 
    } 
    else 
    { 
     fprintf(file, "%s : %s\n", buffer, message); 
    } 

    if(fclose(file) != 0) 
    { 
     printf("Error closing log file.\n"); 
    } 

} 

Die seg Fehler auf der sprintf() Linie geschieht in der logErrorStatus() Funktion.

Jede Hilfe wäre großartig! Vielen Dank!

Antwort

2

Sie versuchen, eine konstante Stringliteral, zu überschreiben, die wahrscheinlich in einem Nur-Lese-Speicherbereich definiert ist, mit einem neuen Wert. Sie sollten stattdessen Ihren Speicher dynamisch zuweisen und freigeben.

Als Beispiel:

void logErrorStatus(char *message) 
{ 
    //ALLOCATE 1024 IN CASE YOU CHANGE THE STRING LITERAL BELOW, AND ADD LENGTH OF MESSAGE 
    char *errorMessage = malloc(1024 + strlen(message)); 

    if (errorMessage != NULL) 
    { 
     sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

     logMessage(errorMessage); 

     free(errorMessage); 
    } 
} 

oder weisen Speicher auf dem Stapel:

void logErrorStatus(char *message) 
{ 
    //ALLOCATE 1024 AS MAX LENGTH OF OUTPUT 
    char errorMessage[1024]; 

    sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

    logMessage(errorMessage); 
} 
+0

prüfen wenn 'malloc'' NULL' zurückgibt oder 'snprintf' verwendet. –

+0

Weitere Überlegungen: Stimmen Sie überein, "versucht, ein konstantes String-Literal zu überschreiben", aber nicht für die Zuweisung von Speicher. Es ist nicht "malloc()" ist böse, es ist, dass eine _error_ Nachricht protokolliert werden soll, Code sollte Ressourcen vermeiden, die typische Fehlerquellen sind. Beispiel, der Fehler ist "Out-of-memory" und diese Antwort ist der Aufruf von 'malloc()' gibt 'NULL' zurück. In diesem Fall besser "logMessage (message);" als nichts zu tun. IMO, favor "" ERROR (% .64s,% d) >>% .80s "," in einen weit genug festen Pufferansatz, da ich nicht übermäßig lange Fehlermeldungen traue. OTOH, diese Antwort erfüllt die Bedenken von OP – chux

+0

@chux - Ich stimme zu. Aber die Frage könnte in Zukunft auch für jemanden nützlich sein, der keinen Fehlerlogger schreibt, und so wurden beide Optionen präsentiert. – owacoder

2

Da errorMessage Punkte auf den Speicher für den "test" Zeichenfolge zugeordnet wörtlichen, die folgende Zeile versucht, nicht beschreibbaren Speicherstelle zu schreiben:

sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

Sie müssen errorMessage in beschreibbaren Speichern zuzuweisen undefiniertes Verhalten zu vermeiden:

char errorMessage[1000]; // 1000 is the max length of the buffer here. 

Dies sollte den Absturzes loszuwerden, aber es ist nicht ideal, da im Extremfall %s s noch den Puffer von 1000ausfließen kanns. Es ist besser, zu zwingen Abschneiden auf beide durch ihre Größe explizit zu beschränken:

sprintf(errorMessage, "ERROR (%64s, %d) >> %900s", __FILE__, __LINE__, message); 
1

"test" ist ein Stringliteral vom Typ char [5] und wird in konstanten Speicher zugewiesen. Obwohl es vom Standard nicht als const definiert ist, können Sie nicht hineinschreiben.

Leider aus dem Nichts hindert Sie es char * machen Eindruck Zuweisung es beschreibbar ist :-(

Das ist, was der Standard sagt über sie:

6.4.5 „Zeichenfolgenliteralen - Semantics“:

5 In der Übersetzungsphase 7 wird an jede Multibyte-Zeichenfolge, die aus einem String-Literal oder Literalen resultiert, ein Byte oder ein Code mit dem Wert Null angehängt.66) Die Multibyte-Zeichenfolge wird dann zum Initialisieren eines Arrays mit statischer Speicherdauer verwendet und lengt h gerade ausreichend, um die Sequenz zu enthalten. Bei Zeichenfolgenliteralen haben die Arrayelemente den Typ char und werden mit den einzelnen Bytes der Multibyte-Zeichenfolge initialisiert.

6 Es ist nicht spezifiziert, ob diese Arrays unterschiedlich sind, vorausgesetzt, ihre Elemente haben die entsprechenden Werte. Wenn das Programm versucht, ein solches Array zu ändern, ist das Verhalten nicht definiert.

GCC nutzt dies, indem sie in R/O-Speicher und das ist, was Dokumentationen darüber sagt:

Modification of string literals is not allowed by the GNU C compiler, because literals are placed in read-only storage.

Alle oben bedeutet, dass Sie ersetzen sollten:

char* errorMessage = "test"; 

sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

mit etwas wie:

char errorMessage[BUFSZ]; 
snprintf(errorMessage, BUFSZ, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

Bitte beachten Sie die Verwendung von snprintf, die der sichere Weg zu printf in Array ist. Es wird den Puffer nicht überschreiben und nicht verwandte Speicherbereiche beschädigen.

0

Dieser Code ist falsch:

char *errorMessage = "test"; 
sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

Hier können Sie errorMessage Punkt auf schreibgeschützt 5 Byte großer statischer Speicher machen (die von "test" + impliziten '\n' belegt ist).

Jeder ordentliche Compiler mit aktivierten Warnungen wird Sie anschreien, wenn Sie versuchen, dem Zeiger auf nicht-kon- stante Zeichen die Adresse eines schreibgeschützten Speichers zuzuweisen.

Sie benötigen also einen veränderbaren Speicher. Sie können dies tun:

char* errorMessage = malloc(256); 
sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 
logMessage(errorMessage); 
free(errorMessage); 

Oder diese:

char errorMessage[256]; 
sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 
logMessage(errorMessage); 

In diesen Beispielen Sie 256 mit einer Größe eines größten Zeichenfolge ersetzen können Sie handhaben zu können wollen.

1

Dieser Code in Ihrer Funktion logErrorStatus

char* errorMessage = "test"; 
sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 
logMessage(errorMessage); 

zuerst definiert einen Zeiger auf eine Stringliteral und versucht dann seine Daten zu überschreiben. Es gibt zwei Probleme mit, dass:

  • Die Stringliteral "test" nicht groß genug ist, um die neuen Daten zu enthalten.
  • String-Literale sind schreibgeschützt, so versuchen, auf sie zu schreiben wird verursachen Probleme.

Ich schlage vor,

char errorMessage[256]; 
snprintf(errorMessage, sizeof errormessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 
logMessage(errorMessage); 
0

Variable errorMessage wird die betroffene Wert Länge festgelegt, in Ihrem Fall „Test“ so 4 zum Beispiel nicht damit zufrieden zu errorMessage wie diese errorMessage[5]= 'o' hinzufügen kann, weil es Coded hart ist Schnur, eine Lösung, den erforderlichen Raum zuzuordnen ist, um die formatierte Nachricht (ERROR (%s, %d) >> %s, selbstverständlich mit der Länge des% s,% D und% s) zu halten:

EDIT :(wie gesagt von @owacoder, wir haben den ungenutzten Speicher free(errorMessage))

void logErrorStatus(char* message) 
{ 
char* errorMessage = (char*)malloc(sizeof(char)*512); 

sprintf(errorMessage, "ERROR (%s, %d) >> %s", __FILE__, __LINE__, message); 

logMessage(errorMessage); 

free(errorMessage); 
} 

befreien, wenn Sie wast Speicher nicht wollen (aber Sie werden in der Leistung verlieren) Sie nur den Platzbedarf wie folgt zuordnen kann:

char* errorMessage = (char*)malloc(sizeof(char)+(14+strlen(__FILE__)+5+strlen(message)+5); 
//14 for message characters => "ERROR (,) >> " 
//5 for the variable __line__ size, it is an int so it's max value is 32767 in standard implementations 
//1 for the \0 charachter 
+0

Ihr erstes Codebeispiel wird leak memory Sie sollten den Speicher "freigeben", wenn er nicht mehr benutzt wird (dh am Ende der Funktion) – owacoder

+0

ja, richtig, korrigiert, danke ^^. –