-1

I'm writing a C program on my mac and I made something basic, but I got this error. I have a struct with a char pointer in it to hold a short description. In my main class I made a pointer to this struct and then another for the char pointer within the struct. When I free the char pointer in the struct, my terminal throws out some error message about a pointer being freed that was not allocated. The error is not present if I comment that line out, so I'm pretty sure it's that line. I'll include the code causing this odd issue. 

Chair *chair = (Chair*)malloc(sizeof(Chair));
chair->color = (char*)malloc(sizeof(char)*4);
chair->color = "blue";
printf("This chair is %s", chair->color);
free(chair->color);
free(chair);
return 0;

When I searched for this, it always came to user error of not mallocing or "double free errors". I'm pretty sure I used malloc and that there isn't a glaringly obvious problem in those 6 lines.

TomWithtime
  • 3
  • 2
  • 2
    `chair->color = "blue";` : `chair->color` rewrite by string literal 's address. (it was not malloc'd address). – BLUEPIXY Nov 16 '14 at 02:45
  • I also just noticed that I don't even need to malloc "strings". My example was so basic that I forgot the basics... I can just use char *string = "some text"; – TomWithtime Nov 16 '14 at 03:04

2 Answers2

3
chair->color = (char*)malloc(sizeof(char)*4);
chair->color = "blue";

To store a 4-char string, you need 5 bytes. 1 extra for the ending \0.
Also, to assign a string, you should use strcpy().

strcpy(chair->color, "blue");
timrau
  • 22,578
  • 4
  • 51
  • 64
0

You have couple of errors.

  1. In the line:

    chair->color = "blue";

    you are assigning chair->color to point to some memory that you didn't get from malloc. Then you are trying to free it later.

    You'll need strcpy to accomplish what you are trying.

    strcpy(chair->color, "blue");

  2. In order to copy "blue", you have to have a string that has at least 5 characters - 4 for the letters and 1 for the terminating null character. You need to use:

    chair->color = malloc(N); // Where N > 4.

    Don't cast the return value of malloc. It is known to be problematic if you do. See Do I cast the result of malloc?.

    You also don't need to use sizeof(char). It is guaranteed to be 1.

Community
  • 1
  • 1
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • I think I just realized the problem. I am coming from a c++ environment, and malloc always gave a void pointer error if it wasn't cast. The chair->color = "blue"; was also doable in that environment. I was writing more c++ than I realized. Thanks for clearing that up! – TomWithtime Nov 16 '14 at 02:58
  • I'm glad I was able to help. – R Sahu Nov 16 '14 at 03:03