2016-04-04 2 views
0

У меня возникли проблемы с определением причины возникновения сегментации в моей программе. Я сделал такие вещи до этого успешно, но эта специфическая функция вызывает ошибку сегментации.Ошибка сегментации - sprintf

Мой код здесь:

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"); 
    } 

} 

вина сегментный происходит на линии Sprintf() в функции logErrorStatus().

Любая помощь будет замечательной! Благодаря!

ответ

2

Вы пытаетесь перезаписать постоянный строковый литерал, который, вероятно, определенные в разделе памяти только для чтения, с новым значением. Вместо этого вы должны динамически распределять и освобождать свою память.

В качестве примера:

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); 
    } 
} 

или выделить память на стеке:

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

Проверить если 'malloc' возвращает' NULL' или использует 'snprintf'. –

+0

Более глубокие соображения: Согласитесь «пытаться перезаписать постоянный строковый литерал», но не в пользу выделения памяти. Это не 'malloc()' is evil, а сообщение _error_ должно регистрироваться, код должен избегать ресурсов, которые являются типичными источниками ошибок. Например, ошибка «Out-of-memory», и этот ответ вызывает вызов 'malloc()' возвращает 'NULL'. Лучше называть 'logMessage (message);' в этом случае, чем ничего не делать. IMO, favor' "ERROR (% .64s,% d) >>% .80s", 'в достаточно широкий подход фиксированного буфера, поскольку я не доверяю чрезмерно длинным сообщениям об ошибках. OTOH, этот ответ отвечает интересам OP – chux

+0

@chux - Я согласен. Но в будущем вопрос может быть полезен и тем, кто не пишет регистратор ошибок, поэтому оба варианта были представлены. – owacoder

2

Поскольку errorMessage указывает на памяти, выделенной для "test" строки буквального, следующая строка пытается записать не-записываемой ячейки памяти:

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

Вам необходимо выделить errorMessage в перезаписываемой памяти, чтобы избежать неопределенного поведения:

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

Это должно избавиться от аварии, но это не является идеальным, потому что в крайних случаях %s s все еще может переполнить буфер 1000 char с. Лучше заставить усечение на них обоих, ограничивая их размер явно:

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

"test" является строка символов типа char [5] и выделяется в постоянной памяти. Хотя он не определен как const по стандарту, вы не можете его записать.

К сожалению, ничто не мешает вам назначать его char * создавая впечатление, что это записываемые :-(

Это то, что стандарт говорит об этом:

6.4.5 «Строковые литералы - Семантика»:

5 В фазе перевода 7 байта или код нулевого значения добавляется к каждой многобайтовой последовательности символов, которая получается из строкового литерала или литералов.66) Последовательность многобайтовых символов затем используется для инициализации массива статической продолжительности хранения и удлинение h достаточно, чтобы содержать последовательность. Для символьных строковых литералов элементы массива имеют тип char и инициализируются отдельными байтами многобайтовой последовательности символов;

6 Не определено, являются ли эти массивы отличными, если их элементы имеют соответствующие значения. Если программа пытается изменить такой массив, поведение не определено.

GCC использует это, сохраняя их в памяти R/O и это то, что Documentations говорит об этом:

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

Все вышесказанное означает, что вы должны заменить:

char* errorMessage = "test"; 

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

с чем-то вроде:

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

обратите внимание на использование snprintf, который является безопасным способом до printf в массив. Он не будет переполнять буфер и повреждать несвязанные области памяти.

0

Этот код некорректен:

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

Здесь вы делаете errorMessage пункт только для чтения 5 байт большой статической памяти (которая занята "test" + неявной '\n').

Любой достойный компилятор с включенными предупреждениями будет кричать на вас, когда вы попытаетесь присвоить адрес хранилища только для чтения указателю на не-const char.

Таким образом, вам необходимо изменить хранилище. Вы можете сделать это:

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

Или это:

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

В этих примерах можно заменить 256 с размером самой большой строки, которую вы хотите, чтобы быть в состоянии справиться.

1

Этот код в функции logErrorStatus

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

первый определяет указатель на строку буквальным, а затем пытается перезаписать данные. Есть две проблемы:

  • Строковый литерал "test" не является достаточно большим, чтобы содержать новые данные .
  • Строковые литералы только для чтения, поэтому попытка написать им будет вызвать проблемы.

Я предлагаю

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

переменная errorMessage фиксируется на пораженной значение длины, в вашем случае «тест» так 4, например, не может добавлять содержимое в errorMessage как этот errorMessage[5]= 'o', потому что трудно закодирована строка, решение состоит в выделение необходимого пространства для хранения отформатированного сообщения (ERROR (%s, %d) >> %s, конечно с длиной% S,% D и% ы):

EDIT :(по словам @owacoder, мы должны освободить неиспользуемую память 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); 
} 

, если вы не хотите, чтобы еси памяти (но вы потеряете в производительности) можно выделить только необходимое место, как это:

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

Ваш первый пример кода будет утечка памяти. Вы должны «освободить» память, когда она больше не используется (т.е. в конце функции). – owacoder

+0

да, правильно, исправлено, спасибо ^^. –

Смежные вопросы