CODEWITHSUNDEEP
cpointerssegmentation-faultfreememcpy
Joseph Kraemer
Joseph Kraemer

Reputation: 111

How do I copy a structure?

I have a problem where I have to copy a structure and add it to new memory then free the old memory. Then I have to increment to create space for new memory and add again. I thought I had the logic correct but I keep getting a segmentation fault. I can't figure out where I am going wrong.

Here is a copy of my structure:

struct childrensBook *book = (struct childrensBook *) malloc(sizeof(struct childrensBook));     //Structure of book #1

book->title = (char *)malloc(100);                  //allows memmory of 100 characters                                                  
book->author = (char *)malloc(100);             //allows memmory of 100 characters  
book->publisher = (char *)malloc(100);              //allows memmory of 100 characters  
book->copyright = 0;
book->price = 0;

Here is a copy of my add function:

int addRecord() 
{
    int headptr = 0;
    struct childrensBook *book = malloc(sizeof(struct   childrensBook));            //get book structure
    struct childrensBook *book1 = malloc(sizeof(struct childrensBook));         //create structure book1
    memcpy(book->title, "We're Going on a Bear Hunt", 26);                      //populate fields of book 
    memcpy(book->author, "Michael Rosen", 13);                                  //populate fields of book 
    memcpy(book->publisher, "Little Simon", 12);                                //populate fields of book 
    book->copyright = 1989;                                                 //populate fields of book 
    book->price = 7.99;                                                     //populate fields of book 
    memcpy(book1, book, sizeof *book1);                     //copy fields of book to book 1
    free(book);
}

And here is my call to function:

else if(x==4)
{
    addRecord();
    fprintf(stderr, "You have added the record: %s\n", book->title);
    free(book);
    moveptr++;   //here to incrememnt for new space. This is a globaal variable
}

Upvotes: 0

Views: 107

Answers (4)

Orest Hera
Orest Hera

Reputation: 6776

In your case the structure struct childrensBook contains pointers to strings. If your want deep copy of that object you need also copy internal strings.

However, since you are using constant lengths for all string fields it is possible to use all string elements as char arrays. In that case the entire content will be stored in one piece of memory:

struct childrensBook {
    char title[100];
    char author[100];
    char publisher[100];
    int copyright;
    int price;
}

/* the structure content may be written as follows */
strcpy(book->title, "We're Going on a Bear Hunt");

/* structure copy by direct assignment */
*book1 = *book;

Upvotes: 0

Jack
Jack

Reputation: 133569

I see multiple errors here:

  • free(book) frees the dynamically allocated struct book but not its dynamically allocated fields (book->title and such things must be freed too)
  • you are calling memcpy to book->author and other char* fields but these fields haven't been dynamically allocated, maybe you wanted to do book->title = strdup("literal"),
  • memcpy(book1, book, sizeof *book1) has at least two errors: first you hare coping sizeof *book1 bytes, which is the size of a pointer, not the whole struct. Then you are copying fields which doesn't contain primitive types but pointers which would then share ownership between two books, you should copy each single dynamically allocated field.

Just to give some code to start with:

void free_book(struct book* b) {
  free(b->title);
  free(b->author);
  ...
  free(b);
}

struct* book dupe_book(struct book* source) {
  struct book* dest = malloc(sizeof(struct book));
  dest->price = source->price;
  dest->author = strdup(source->author);
  ...
}

Upvotes: 2

ameyCU
ameyCU

Reputation: 16607

memcpy do not add null terminator. So either add '\0' after memcpy manually or increase number of byte in memcpy .

Do this - 

memcpy(book->title, "We're Going on a Bear Hunt", 26); // or use strncpy so it will add null terminator          
memcpy(book->author, "Michael Rosen", 14);                                   
memcpy(book->publisher, "Little Simon", 13);

And because your book->title is not null terminated , therefore , fprintf with specifier %s gives seg fault.

And if you use fixed memory in malloc(i.e 100) , why use it ? Declare them as arrays -

book->title = (char *)malloc(100);          // in struct declare title as char title[100]             
book->author = (char *)malloc(100);         // similar with rest.     
book->publisher = (char *)malloc(100);

Then you won't have take tension to free it.

Note - If you use malloc then as also mentioned in comment , please don't cast result of malloc.

Upvotes: 1

Bathsheba
Bathsheba

Reputation: 234725

Don't forget to copy over the null-terminators on your string literals:

You need to use "Little Simon", 13 instead etc.: "Little Simon" is not as little as you think. It occupies 13 bytes, not 12.

Else your fprintf will overrun past your string, with disastrous results: a segmentation fault being one possibility.

Upvotes: 0

Related Questions