Ошибка сегментации при анализе содержимого строки, разделенной разделителем

Во время интервью меня попросили реализовать решение следующей задачи на языке программирования C:

Учитывая входную строку с фиксированным шаблоном содержимого, разделенным разделителем, мне нужно извлечь каждое конкретное содержимое.

Шаблон входной строки: начальное_сообщение|целое_значение_1|целое_значение_2|код_символа|конечное_сообщение

Ожидаемый результат:

Starting message: starting_message
Value 1 : integer_value_1
Value 2 : integer_value_2
Char code : character_code
Ending message : ending_message

Пример ввода: HelloWorld|35|45|C|ByeWorld

Пример вывода:

Starting message: HelloWorld
Value 1 : 35
Value 2 : 45
Char code : C
Ending message : ByeWorld

Я реализовал следующий код:

#include<stdio.h>
#include<string.h>
#include<stdlib.h>
/*
Loop through the input string until termination.
Figure out the position of delimiters first. It will help in parsing later.
For string, count the number of chars and use memcpy for that number of elements.
To parse char to int, try atoi.
*/

void parse(const char* input, char* starting_message, int* value1, int* value2, char* char_code, char* ending_message)
{
  int ctr = 0, pos_ctr=0;
  int delim_pos[4];  /* To store the location of delimiters aka '|' */
  
  char* value1_str = (char*) malloc(10);
  char* value2_str = (char*) malloc(10);
  
  while(input[ctr]!= '\0')
  {
    if(input[ctr] == '|')
    {
      if(pos_ctr < 4)
      {
        delim_pos[pos_ctr] = ctr;
        pos_ctr++;
      }
    }
    ctr++;
  }
  
  memcpy(starting_message,input,(delim_pos[0]));  /* starting_message is contained in input string in between input[0] & input[delim_pos[0]]*/
  starting_message[delim_pos[0]+1] = '\0';
  
  memcpy(value1_str, input,(delim_pos[1]-delim_pos[0]));   /* value1 is contained in input string in between input[delim_pos[0]] & input[delim_pos[1]]*/
  value1_str[(delim_pos[1]-delim_pos[0] + 1)] = '\0';
  *value1 = atoi(value1_str);
  
  memcpy(value2_str, input,(delim_pos[2]-delim_pos[1]));   /* value2 is contained in input string in between input[delim_pos[1]] & input[delim_pos[2]]*/
  value1_str[(delim_pos[2]-delim_pos[1] + 1)] = '\0';
  *value2 = atoi(value2_str);
  
  *char_code = input[(delim_pos[2]+1)];     /* char_code is element next to input[delim_pos[2]]*/
  
  memcpy(ending_message, input, (delim_pos[3]-ctr-1));  /* ending_message is contained in input string in between input[delim_pos[3]] & null termination char*/
  ending_message[delim_pos[3]-ctr] = '\0';
  
}


int main() {

const char* input = "HelloWorld|35|45|C|ByeWorld";
char* starting_message = (char*) malloc(30);
char* ending_message = (char*) malloc(30);
int value1, value2;
char char_code;

parse(input, starting_message, &value1, &value2, &char_code, ending_message);


printf(" Input string: %s\n",input);
printf("Starting message : %s\n", starting_message);
printf("Value 1 : %d\n", value1);
printf("Value 2 : %d\n", value2);
printf("Character code : %c\n", char_code);
printf("Ending message: %s\n", ending_message);

    return 0;
}

Я смог скомпилировать, но на экране вывода я столкнулся с ошибкой сегментации без вывода.

Экран вывода

Где я ошибся и как это исправить?


person Kshitij_9192    schedule 02.03.2021    source источник
comment
Мой совет: никогда не делайте никаких тестов на собеседовании. Собеседование — это не экзамен. Это разговор. :)   -  person Vlad from Moscow    schedule 02.03.2021
comment
Обратите внимание, что нет необходимости выделять буферы для строк, переданных в atoi. Его можно вызвать непосредственно во входной строке (плюс смещение до первого символа значения). Например. *value1 = atoi(&input[delim_pos[0]+1]);.   -  person Ian Abbott    schedule 02.03.2021
comment
@ Kshitij_9192 Что касается функции, то она не имеет большого смысла. Например, функция не сообщает, была ли обработка успешной или нет.   -  person Vlad from Moscow    schedule 02.03.2021
comment
Другая ошибка заключается в том, что все вызовы memcpy копируются с начала входной строки.   -  person Ian Abbott    schedule 02.03.2021
comment
@Kshitij_9192 Эти выделения памяти char* value1_str = (char*) malloc(10); char* value2_str = (char*) malloc(10); являются избыточными. Более того, функция производит утечки памяти. :)   -  person Vlad from Moscow    schedule 02.03.2021


Ответы (2)


Ваша ошибка сегментации возникает здесь:

memcpy(ending_message, input, (delim_pos[3]-ctr-1));

Типичными причинами ошибок сегментации являются пустые указатели или указатели на мусор, но здесь дело в другом: вы неправильно рассчитали количество байтов для копирования, потому что неправильно выполнили вычитание. Так должно быть

ctr + 1 - delim_pos[3]

потому что, если все прошло хорошо, str прошло delim_pos[3]. (Но вы не проверяете это pos_ctr == 4, так что вы не знаете, все ли прошло хорошо.)

Третий параметр memcpy имеет беззнаковый целочисленный тип size_t. Перепутав вычитание, вы получили небольшое отрицательное значение int, которое будет огромным положительным значением при преобразовании в значение без знака, что, безусловно, приведет к доступу за пределы.

person M Oehm    schedule 02.03.2021
comment
Спасибо @M Oehm, это была глупая ошибка с моей стороны. - person Kshitij_9192; 02.03.2021

Во-первых, функция не сообщает, был ли разбор входной строки успешным.

Функция имеет слишком много параметров. Вместо этого списка параметров

char* starting_message, int* value1, int* value2, char* char_code, char* ending_message

вы можете передать объект структурного типа. Например, объявление функции может быть

int parse( const char *input, struct Fields *fields );

Эти выделения памяти

char* value1_str = (char*) malloc(10);
char* value2_str = (char*) malloc(10)

являются избыточными и, кроме того, из-за этих распределений функция производит утечки памяти.

Вместо функции atoi следует использовать функцию strtol.

Вместо этого цикла while

while(input[ctr]!= '\0')

вы можете использовать цикл, внутри которого вызывается функция strchr.

В этом задании

starting_message[delim_pos[0]+1] = '\0';

символ в позиции delim_pos[0] может иметь неопределенное значение. Вместо утверждения выше вы должны написать

starting_message[delim_pos[0]] = '\0';

В этих заявлениях

memcpy(value1_str, input,(delim_pos[1]-delim_pos[0]));
memcpy(value2_str, input,(delim_pos[2]-delim_pos[1]));
memcpy(ending_message, input, (delim_pos[3]-ctr-1)); 

вы копируете символы, начиная с начала строки input, что не имеет смысла. Более того, это выражение delim_pos[3]-ctr-1 даст отрицательное значение.

Это утверждение

ending_message[delim_pos[3]-ctr] = '\0';

вызывает неопределенное поведение, поскольку значение выражения delim_pos[3]-ctr отрицательное. ctr хранит длину входной строки.

Таким образом, функция имеет неопределенное поведение и не имеет большого смысла. :)

person Vlad from Moscow    schedule 02.03.2021
comment
Спасибо @Vlad за такое подробное объяснение и объем улучшений в этой программе. Благодаря вашим подробным комментариям я смог понять, что эта программа — грубая ошибка. :) - person Kshitij_9192; 02.03.2021