Копирование со строкой в ​​связанный список не работает

Question

У меня есть задача скопировать некоторые входные данные в связанные списки, но когда я пытаюсь скопировать это с помощью strncpy, это не работает, и я получил ошибку

Exception thrown at 0x0F4C0E15 (ucrtbased.dll) in 
ProjectA.exe: 0xC0000005: Access violation writing location 0xCDCDCDCD.

Код:

typedef struct Frame
{
    char*       name;
    unsigned int    duration;
    char*       path;  
} Frame;

typedef struct FrameNode
{
    Frame* frame;
    struct FrameNode* next;
} FrameNode;

FrameNode* createframe(char name[], char path[], int duration)
{
    Frame* list = (Frame*)malloc(sizeof(Frame));
    strncpy((list->name), name, STR_LEN);
    list->duration = duration;
    strncpy(list->path, path, STR_LEN);
    FrameNode* frame = list;
    frame->next = NULL;
    return list;
}

Answer 1

Базовая диагностика и рецепт

После malloc(), list->name является неинициализированным указателем.Вам нужно выделить достаточно места для строки, а затем скопировать в это пространство.Аналогично с path.Не забудьте учесть нулевые байты на концах строк.Вы не выделяете место для FrameNode;вы также не вернете его.

FrameNode *createframe(char name[], char path[], int duration)
{
    Frame *list = (Frame*)malloc(sizeof(*list));
    size_t name_len = strlen(name) + 1;
    char  *name_cpy = malloc(name_len); 
    size_t path_len = strlen(path) + 1;
    char  *path_cpy = malloc(path_len);
    FrameNode *frame = malloc(sizeof(*frame));
    if (list == NULL || name_cpy == NULL || path_cpy == NULL || frame == NULL)
    {
        free(name_cpy);
        free(path_cpy);
        free(frame);
        free(list);
        return NULL;
    }
    list->duration = duration;
    memmove(path_cpy, path, path_len);
    memmove(name_cpy, name, name_len);
    list->name = name_cpy;
    list->path = path_cpy;
    frame->frame = list;
    frame->next = NULL;
    return frame;
}

В нем много исправлений.

• Он выделяет место для Frame и FrameNode.
• Код проверяет ошибки выделения.
• Он пытается выделить всю память перед проверкой ошибок, что упрощает обработку ошибок.Там только одна ошибка возврата.Если первое распределение завершится неудачно, скорее всего, остальное тоже будет, инициализируя переменные равными NULL, которые можно безопасно передать в free().
• Он вычисляет длину строк.
• Он выделяет место для строк и их нулевого терминатора.
• Поскольку он знает, как долго строки, он может использовать memmove() (или memcpy()) для копирования данных.
• Используется нотация sizeof(*variable) вместо sizeof(VariableType).
• Возвращается FrameNode * вместо Frame *.
• . Избегает использования strncpy(), поскольку это не гарантируетскопированная строка завершается нулем, что приводит к проблемам в другом месте.
• Не обращает внимания на STR_LEN - неясно, какое значение имеет.

Альтернативапроекты для структур

Если у вас есть фиксированный верхний предел размера name и path, вам лучше работать со структурой, подобной:

typedef struct Frame
{
    unsigned int    duration;
    char            name[STR_LEN + 1];
    char            path[STR_LEN + 1];  
} Frame;

• Используйте фиксированный размер элементов для сохранения выделения.
• Разрешить для строк length STR_LEN плюс нулевой терминатор.
• Поместите символьные массивы в конец структуры, чтобы минимизировать заполнение, независимо от значения STR_LEN.
• Затем вам потребуетсяиспользуйте strncpy() и убедитесь, что вы установили list->name[STR_LEN] = '\0'; и list->path[STR_LEN] = '\0'; - и именно поэтому в определениях элементов есть + 1.

Это изменение уменьшает количество выделений с 4 до2. Вы даже можете включить указатель next в структуру Frame и вообще покончить со структурой FrameNode - снова уменьшив объем необходимого управления памятью и, следовательно, упростив код.Также могут быть веские причины отделять FrameNode от Frame, но это не ясно из информации в вопросе - и не обязательно;это то, что вы должны рассмотреть, вот и все.

Answer 2

Вам нужно выделить место с malloc перед копированием строки в месте назначения. Первый malloc выделяет только пространство для Frame, но не для его внутренних char *.

Код в вашем createframe должен быть:

    Frame* list = malloc(sizeof(Frame));
    list->name = malloc(STR_LEN);
    strncpy((list->name), name, STR_LEN);
    list->duration = duration;
    list->path= malloc(STR_LEN);
    strncpy(list->path, path, STR_LEN);
    //FrameNode* frame = list; // <- nope. FrameNode* should point to a FrameNode not a Frame
    FrameNode* frame = malloc(sizeof(FrameNode)); 
    frame->frame = list;
    frame->next = NULL;
    return frame;

Также было бы хорошо проверить, что malloc успешно, прежде чем использовать динамически распределенные переменные, например:

 Frame *list = malloc(sizeof(Frame));
 if(list==NULL){
     perror("problem allocating frame");
     return NULL;
 }
 list->name = malloc(STR_LEN);
 if(list->name==NULL){
     free(list);//free the already allocated memory
     perror("error message");
     return NULL;
  }
 strncpy((list->name), name, STR_LEN);
 ...
 return frame;
 }

Когда возвращается createframe, вы должны проверить, вернул ли он NULL или нет, и в случае, если он вернул NULL, обработать ошибку, обычно путем освобождения выделенной памяти и завершения программы.

---

Вы не должны разыгрывать результат malloc

Answer 3

Для начала функция должна быть объявлена ​​как

FrameNode * createframe( const char name[], const char path[], int duration );

потому что ни name, ни path не изменены в функции.

Вы не выделили память для list->name, list->path и frame.

И, кроме того, вместо frame типа FrameNode * функция возвращает list типа Frame *.

Именованная константа STR_LEN не имеет большого смысла, когда массив символов выделяется динамически, как в вашем случае с элементами данных name и path структуры Frame.

Сначала вы должны динамически выделить память для объекта типа FrameNode.

Затем вы должны выделить память для объекта типа Frame и его элементов данных name и path.

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

FrameNode * createframe( const char name[], const char path[], int duration )
{
    FrameNode *frame = malloc( sizeof( FrameNode ) );

    if ( frame != NULL )
    {
        char *name_data = NULL;
        char *path_data = NULL;

        size_t n = strlen( name );

        name_data = malloc( n + 1 );

        if ( name_data != NULL ) strcpy( name_data, name );

        if ( name_data != NULL )
        {
            n = strlen( path );

            path_data = malloc( n + 1 );

            if ( path_data != NULL ) strcpy( path_data, path );   
        }

        Frame *list = NULL;

        if ( name_data != NULL && path_data != NULL )
        {
            list = malloc( sizeof( Frame ) );

            if ( list != NULL )
            {
                list->name     = name_data;
                list->duration = duration;
                list->path     = path_data;
            }
        }

        if ( list == NULL )
        {
            free( name_data );
            free( path_data );
            free( frame );
        }
        else
        {
            frame->frame = list;
            frame->next  = NULL; 
        }
    }        

    return frame;
}

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

size_t n = strlen( name );

name_data = malloc( n + 1 );

if ( name_data != NULL ) strcpy( name_data, name );

и

n = strlen( path );

path_data = malloc( n + 1 );

if ( path_data != NULL ) strcpy( path_data, path );   

следует заменить на

name_data = malloc( STR_LEN );

if ( name_data != NULL ) 
{
    strncpy( name_data, name, STR_LEN );
    name_data[STR_LEN - 1] = '\0';
}           

и

path_data = malloc( STR_LEN );

if ( path_data != NULL ) 
{
    strncpy( path_data, path, STR_LEN );   
    path_data[STR_LEN - 1] = '\0';
}               

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