Null String when inserting characters (C)-CodePudding

I am currently working on a text editing program in C, which uses Linked Lists for rows of text. I have so far written functions for resizing the list etc., but I have now attempted to write the insert_char(Row* row, int idx, char c) however whenever I try resizing it, the resulting char* array is NULL. I am confident it's not a memory leak, as I have checked and I am free()ing all of my malloc()'d memory, so I really don't know where the problem is.

I have also tried some printf("%c", c) debugging to view the character, however the character itself is also NULL. Can anyone help me with this?

Here is the struct for a Row:

typedef struct {
    char* data; // pointer to Malloc()'d char array.
    int datalen;
} Row;

Here are the functions for resizing the row and allocating the Row pointer.

Row* alloc_row(char* data)
{
    Row* row = (Row*) malloc(sizeof(Row));
    char* data2 = (char*) malloc((sizeof(char) * strlen(data)) 1);
    strcpy(data2, data);
    row->data = data2;
    row->datalen = strlen(data);
    return row;
}

// Row resize

Row* resize_row(Row* oldrow, char* data)
{
    Row* new_row = (Row*) malloc(sizeof(Row));
    new_row->data = data;
    new_row->datalen = strlen(data);

    // free() the old row
    free(oldrow->data);
    free(oldrow);

    return new_row;
}

And here is the function I am having trouble with - it should take a Row*, create a buffer, strcpy() the Row->data up to idx, insert the char c and then copy the rest of the string afterwards, such that if I called alloc_row(Row* {.data = "Hello" .strlen=5}, 2, 'A') I would receive HeAllo (counting from zero). However, the string is NULL:

Row* insert_char(Row* row, int idx, char c)
{
    char* new_row = (char*)malloc(sizeof(char) * (strlen(row->data)   2)); // 1 char for null, char for the appended data
    if (idx < strlen(row->data)) {
        for (int i = 0; i < strlen(row->data) 1; i  ) {
            if (i < idx) new_row[i] = row->data[i];
            if (i == idx) new_row[idx] = c;
            if (i > idx) new_row[i] = row->data[i-1];
        }
    } else {
        row->data[strlen(row->data)] = '\0';
        strncpy(new_row, row->data, strlen(row->data));
        new_row[strlen(row->data)-1] = c;
    }
    Row* nr = resize_row(row, new_row);
    return nr;
}

Is there something wrong with my approach, and is there a cleaner and faster way of doing this?

CodePudding user response：

I tried the following code and it works (I modified certain things to print it directly and corrected some of your suggestions on how to call the function):

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct {
    char* data; // pointer to Malloc()'d char array.
    int datalen;
} Row;

char* insert_char(Row* row, int idx, char c)
{
    char* new_row = (char*)malloc(sizeof(char) * (strlen(row->data)   2)); // 1 char for null, char for the appended data
    if (idx < strlen(row->data)) {
        for (int i = 0; i < strlen(row->data) 1; i  ) {
            if (i < idx) new_row[i] = row->data[i];
            if (i == idx) new_row[idx] = c;
            if (i > idx) new_row[i] = row->data[i-1];
        }
    } else {
        row->data[strlen(row->data)] = '\0';
        strncpy(new_row, row->data, strlen(row->data));
        new_row[strlen(row->data)-1] = c;
    }
    return new_row;
}


int main()
{
    printf("%s\n", insert_char(&(Row) {.data = "Hello", .datalen=5}, 2, 'A'));

    return 0;
}

However, I think that your problem is in the for where you need 2 instead of 1 in the ending condition (since you are copying the entire array and malloc doesn't necessarly set the last char as '\0' [although calloc could do that]).

CodePudding user response：

At least this problem:

new_row[] is not a string as it lacks a null character. Later code relies on that.
Result: undefined behavior (UB).

char* new_row = (char*)malloc(sizeof(char) * (strlen(row->data)   2));
if (idx < strlen(row->data)) {
  ...
} else {
    row->data[strlen(row->data)] = '\0';
    strncpy(new_row, row->data, strlen(row->data));
    // At this point `new_row[]` lacks a '\0'
    new_row[strlen(row->data)-1] = c;
}

It is unclear exactly what OP's wants in the else block, but I think it may be:

} else {
    size_t len = strlen(row->data);
    strcpy(new_row, row->data);
    new_row[len  ] = c;
    new_row[len] = '\0';
}