CODEWITHSUNDEEP
cmalloc
user2635911
user2635911

Reputation: 492

Can't understand why I'm getting segmentation fault due to strncpy

I am getting a segmentation fault when I use strncpy and I cannot figure out how to fix it.

Here is my rectangle.h file. This is just a header file.

#define NAMESIZE 20

struct point {
    int x;
    int y;
};

struct rectangle {
    struct point upperleft;
    struct point lowerright;
    //char label[NAMESIZE + 1];
    char *label;
};

struct point *create_point(int x, int y);

struct rectangle *create_rectangle(struct point ul, struct point lr, 
                                   char *label);

int area1(struct rectangle r);
int area2(struct rectangle *r);
void change_label(struct rectangle *r, char *newlabel);
void broken_change_label(struct rectangle r, char * newlabel);
void print_rectangle(struct rectangle *r);

This is my rectangle.c file. I am just going to show one function from it:

/* create_rectangle dynamically allocates memory to store a rectangle, gives it * initial values, and returns a pointer to the newly created rectangle. */

struct rectangle *create_rectangle(struct point ul, struct point lr, 
                                   char *label) {

    struct rectangle *r = malloc(sizeof(struct rectangle));
    /* TASK 1: fill in the rest of this function */
    r->upperleft = ul;
    r->lowerright = lr;
    r->label = malloc(strlen(label) * sizeof(char));
    strncpy(r->label, label, strlen(label) + 1);

    return r;
}

In the above code, I use malloc to allocate enough space for the label pointer.

When I run the tester.c program below, I get a segmentation fault error (due to the strncpy).

int main(void) {    

    char *str1 = "Big rectangle";
    char *str2 = "Square";

    struct point *p1 = create_point(10, 10);
    struct point *p2 = create_point(100, 100);

    struct rectangle *r1 = create_rectangle(*p1, *p2, str1);
    print_rectangle(r1);
    printf("    expecting: (10, 10) (100, 100) Big rectangle\n");

    free(p2);
    p2 = create_point(20, 20);

    struct rectangle r2;

    strncpy(r2.label, str2, NAMESIZE); //GETTING SEGMENTATION FAULT DUE TO THIS LINE
}

I suspect that I am getting a segmentation fault error because I am doing strncpy directly on r2.label. I suspect that because I am not allocating any space to char *label in the rectangle struct is why I am getting a segmentation fault error. But when I write

char *label = malloc(sizeof(NAMESIZE) * sizeof(char));

I get an error:

error: expected ';' at end of declaration list` error.

Upvotes: 0

Views: 150

Answers (5)

M.M
M.M

Reputation: 141554

This code:

r->label = malloc(strlen(label) * sizeof(char));
strncpy(r->label, label, strlen(label) + 1);

should be:

r->label = malloc( strlen(label) + 1 );
strcpy( r->label, label );
  • You need to allocate space for the null terminator.
  • sizeof(char) is always 1
  • The strcpy function finishes after writing strlen(label)+1 bytes, so it is redundant to try and use strncpy.

strncpy is fairly dangerous in general because sometimes it does not output a string; my advice would be to never use it.

Further down,

strncpy(r2.label, str2, NAMESIZE);

is wrong because r2.label is currently a wild pointer. You also need to allocate memory in the same way:

r2.label = malloc( strlen(str2) + 1 );
strcpy( r2.label, str2 );

Upvotes: 2

ETFovac
ETFovac

Reputation: 111

r->label = malloc(strlen(label) * sizeof(char));

You are mising allocation for terminating character, it should be

r->label = malloc((strlen(label)+1) * sizeof(char));

Upvotes: 0

sherrellbc
sherrellbc

Reputation: 4853

Look like, as you said, you aren't allocating any memory for your char* label.

Do:

r2.label = malloc(sizeof(NAMESIZE) * sizeof(char));`

Upvotes: 0

Yu Hao
Yu Hao

Reputation: 122383

strncpy(r2.label, str2, NAMESIZE);

You are trying to write to r2.label, which is a pointer that hasn't been allocated any space.

And be careful with the code that you did allocate space:

r->label = malloc(strlen(label) * sizeof(char));

strlen(label) is not enough, a string needs to be null-terminated.

Upvotes: 4

Scott Hunter
Scott Hunter

Reputation: 49803

You malloc strlen(label) bytes for the label in create_rectangle, then copy in strlen(label)+1 bytes. Note that, if the new label you are copying in is longer, you'll overflow that allocation. But as @YuHao points out, it doesn't matter, because you never initialized any of the fields of r2 (in particular, label).

Upvotes: 0

Related Questions