Search code examples
cpointerssegmentation-faultstrcatstrstr

Segfault when allocating memory to a struct


I want to concatenate using strcat each new tag stored in a struct with char member in a the char tags_cell_concat. I get a segfault

First attempt


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

#define BUFDATE 50
#define BIGBUFFLEN 1024

typedef struct 
{
  char date[BUFDATE];
  char tags[BUFDATE];
  char task[BUFDATE];
  char next_step[BUFDATE];
} Record;

int main()
{
  Record *records = (Record *) malloc(2*sizeof(Record));
  // all tags: "Hello world I am a string slice function"
  strcpy(records[0].tags, "Hello world");
  strcpy(records[1].tags, "string slice");
  strcpy(records[2].tags, "function");

  char tags_cell_concat[BIGBUFFLEN] = {0};
  char *chunk;
  char *pointer;

  for (size_t i=0; i<3; i++){
    strcat(tags_cell_concat, " ");
    pointer = records[i].date;
    while ((chunk = strsep(&pointer, " ")) !=NULL)
    {
      printf("%s\n", chunk);
    }
    if (strstr(tags_cell_concat, chunk) != NULL)
    {
      strcat(tags_cell_concat, chunk);
    }
  }
  return 0;
}

I get a Segmentation fault at runtime

EDIT 1

After the comments and answer below, there is indeed a first problem with the code above (which I did not change) in that there was not enough space allocated for records. I changed the code to allocate enough memory but the problem persists. The seg fault problem goes away if the line 36-39 are commented out which indicates to me that the problem might lie with something with the use of the strcat() function:

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

#define BUFDATE 50
#define BIGBUFFLEN 1024

typedef struct 
{
  char tags[BUFDATE];
} Record;

int main()
{
  size_t size = 6;
  Record *records = (Record *) malloc(size*sizeof(Record));
  strcpy(records[0].tags, "tag1 tag2");
  strcpy(records[1].tags, "tag3");
  strcpy(records[2].tags, "tag4 tag2");
  strcpy(records[3].tags, "tag1 tag5");
  strcpy(records[4].tags, "tag6 tag3");
  strcpy(records[5].tags, "tag7 tag2");
  strcpy(records[6].tags, "tag8 tag5");

  char tags_cell_concat[BIGBUFFLEN] = {0};
  char *chunk;
  char *pointer;

  for (size_t i=0; i<size; i++){
    strcat(tags_cell_concat, " ");
    pointer = records[i].tags;
    while ((chunk = strsep(&pointer, " ")) !=NULL)
    {
      printf("%s\n", chunk);
    }
    if (strstr(tags_cell_concat, chunk) != NULL) // line to comment out and get no seg fault
    { // line to comment out and get no seg fault
      strcat(tags_cell_concat, chunk); // line to comment out and get no seg fault
    } // line to comment out and get no seg fault
  }
  return 0;
}


This is the output now:

tag1
tag2
make: *** [runc] Segmentation fault: 11

Edit 2

I again changed the code in that I removed one Record in record so that it corresponds to allocated memory. Additionnaly I check whether chunk is not NULL in the if statement (with (chunk !=NULL) is it correct?). But the problem still persists. Here is the last version of the code:

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

#define BUFDATE 50
#define BIGBUFFLEN 1024

typedef struct 
{
  char tags[BUFDATE];
} Record;

int main()
{
  size_t size = 6;
  Record *records = (Record *) malloc(size*sizeof(Record));
  strcpy(records[0].tags, "tag1 tag2");
  strcpy(records[1].tags, "tag3");
  strcpy(records[2].tags, "tag4 tag2");
  strcpy(records[3].tags, "tag1 tag5");
  strcpy(records[4].tags, "tag6 tag3");
  strcpy(records[5].tags, "tag7 tag2");

  char tags_cell_concat[BIGBUFFLEN] = {0};
  char *chunk;
  char *pointer;

  for (size_t i=0; i<size; i++){
    strcat(tags_cell_concat, " ");
    pointer = records[i].tags;
    while ((chunk = strsep(&pointer, " ")) !=NULL)
    {
      printf("%s\n", chunk);
    }
    if ((strstr(tags_cell_concat, chunk) != NULL) && (chunk !=NULL)) // line to comment out and get no seg fault
    { // line to comment out and get no seg fault
      strcat(tags_cell_concat, chunk); // line to comment out and get no seg faultgg
    } // line to comment out and get no seg fault
  }
  return 0;
}

Final edit

Here is finally what worked for me (which is not the topic of the answer I originally posted and therefore does not constitue a valid answer). The bug was due to the condition if (strstr(tags_cell_concat, chunk) != NULL) which was changed into if (strstr(tags_cell_concat, chunk) == NULL)


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

#define BUFDATE 50
#define BIGBUFFLEN 1024

typedef struct 
{
  char tags[BUFDATE];
} Record;

int main()
{
  size_t size = 6;
  Record *records = (Record *) malloc(size*sizeof(Record));
  strcpy(records[0].tags, "tag1 tag2");
  strcpy(records[1].tags, "tag3");
  strcpy(records[2].tags, "tag4 tag2");
  strcpy(records[3].tags, "tag1 tag5");
  strcpy(records[4].tags, "tag6 tag3");
  strcpy(records[5].tags, "tag7 tag2");

  char tags_cell_concat[BIGBUFFLEN] = {0};
  char *chunk;
  char *pointer;

  for (size_t i=0; i<size; i++){
    pointer = records[i].tags;
    while ((chunk = strsep(&pointer, " ")) !=NULL)
    {
      if (strstr(tags_cell_concat, chunk) == NULL)
      { 
        strcat(tags_cell_concat, chunk);
        strcat(tags_cell_concat, " ");
      } 
    }
  }
  printf("tags_cell_concat:%s:\n", tags_cell_concat);
  return 0;
}

Solution

  • You allocated memory only to an array of two elements of the type Record.

      Record *records = (Record *) malloc(2*sizeof(Record));
    

    So tрe valid range of indices for accessing elements of the array is [0, 1].

    However you are using the invalid index 2 to access an element of the array

      // all tags: "Hello world I am a string slice function"
      strcpy(records[0].tags, "Hello world");
      strcpy(records[1].tags, "string slice");
      strcpy(records[2].tags, "function");
    

    that results in undefined behavior.

    And the condition in this for loop

     for (size_t i=0; i<3; i++){
    

    is also invalid.

    One of reasons of the errors is that you are using magic numbers like 2 and 3. Instead use named constants.

    Also not all data members of elements of the array were initialized. So these statements

    pointer = records[i].date;
    while ((chunk = strsep(&pointer, " ")) !=NULL)
    {
      printf("%s\n", chunk);
    }
    if (strstr(tags_cell_concat, chunk) != NULL)
    {
      strcat(tags_cell_concat, chunk);
    }
    

    also invoke undefined behavior.