How to properly malloc for array of struct in C

Question

I will read in two set of char* (or strings) using strtok, and since those two set of chars are related, (address : command\n) I decided to use a structure.

struct line* array = (struct line*)malloc(sizeof(file) * sizeof(struct line*));

This line mallocing space for the function gives me a segmentation fault and was wondering if you can tell me a proper way to malloc space for it. For context, here is the rest of my code:

struct line
{
    char* addr;
    char* inst;
};
while loop{
    x = strtok(line,": ");
    y = strtok(NULL,"\n");
    strcpy(array[i].addr,x); //assume that x and y are always 3characters
    strcpy(array[i].inst,++y);
    i++;
}

Answer 1

For what you have described, You do not need to allocate memory for your struct, rather, you need to allocate memory for the members char *addr;, and char *inst;. If you want to have a single copy of that structure, the first section of code illustrates how to initialize, and assign values. If you want an array, the second code example illustrates the differences.

This illustrates how to allocate memory for the members of a single struct line:

typedef struct
{
    char* addr;
    char* inst;
}LINE;

LINE line;  

int main(void)
{   

    strcpy(line.addr, "anystring"); //will fail
    line.addr = malloc(80);
    line.inst = malloc(80);
    strcpy(line.addr, "someString");//success;
    strcpy(line.inst, "someOtherString");//success;

}

For array of struct line...

typedef struct
{
    char* addr;
    char* inst;
}LINE;  //same struct definition

LINE line[10]; //but create an array of line here.

int main(void)
{   
    int i;
    
    for(i=0;i<10;i++)
    {
      line[i].addr = malloc(80);
      line[i].inst = malloc(80);
    }

    for(i=0;i<10;i++)
    {
        strcpy(line[i].addr, "someString");
        strcpy(line[i].inst, "someOtherString");
    }
    //when done, free memory
    for(i=0;i<10;i++)
    {
        free(line[i].addr);
        free(line[i].inst);
    }      


}

Added to address comment
Addressing the comment under this answer from @Adam Liss, the following code illustrates the following improvements using strdup(): 1) Uses only memory needed. 2) Performs memory creation and copy operations in one step, so the the following blocks:

for(i=0;i<10;i++)
{
  line[i].addr = malloc(80);
  line[i].inst = malloc(80);
}

for(i=0;i<10;i++)
{
    strcpy(line[i].addr, "someString");
    strcpy(line[i].inst, "someOtherString");
}

Become:

for(i=0;i<10;i++)
{
  line[i].addr = strdup("someString");
  line[i].inst = strdup("someOtherString");
}

One more note: Error handling was not included in examples above to avoid muddling up focus on the main concepts: But for the sake of completeness, because both malloc() and strdup() can fail, actual usage for each of these two functions, should include a test before using, eg:

Rather than

  line[i].addr = strdup("someString");
  line[i].inst = strdup("someOtherString");

The code should include

  line[i].addr = strdup("someString");
  if(!line[i].addr)
  {
      //error handling code here
  }
  line[i].inst = strdup("someOtherString");
  if(!line[i].inst)
  {
      //error handling code here
  }

Answer 2

Allocating works the same for all types. If you need to allocate an array of line structs, you do that with:

struct line* array = malloc(number_of_elements * sizeof(struct line));

In your code, you were allocating an array that had the appropriate size for line pointers, not for line structs. Also note that there is no reason to cast the return value of malloc().

Note that's it's better style to use:

sizeof(*array)

instead of:

sizeof(struct line)

The reason for this is that the allocation will still work as intended in case you change the type of array. In this case this is unlikely, but it's just a general thing worth getting used to.

Also note that it's possible to avoid having to repeat the word struct over and over again, by typedefing the struct:

typedef struct line
{
    char* addr;
    char* inst;
} line;

You can then just do:

line* array = malloc(number_of_elements * sizeof(*array));

Of course don't forget to also allocate memory for array.addr and array.inst.