user5514267
user5514267

Reputation: 31

Invalid read of size 1 Valgrind error

In my program, the function read_commands just gets 2 strings and puts it into the struct Commands and returns the filled struct.
Apparently I have some fault in my logic. I'm getting the error:

Invalid read size of 1

In valgrind.

typedef struct{  
Command *test  
Command *compile
}Commands;


typedef struct Command{  
char *command;  
struct Command *next;
}Command;

I just do read_commands("abc", "abc");

The rest of my code:

Commands read_commands(const char *compile_cmds, const char *test_cmds) {
  Commands commands;
  Command *compile, *test;
  int i;              
  if (compile_cmds == NULL || test_cmds == NULL)
     exit(0);

  compile = commands.compile;
  test = commands.test;

  i = 0;
  while (compile_cmds + i != NULL) {
    compile = malloc(sizeof(Command));
    compile->command = malloc(strlen((compile_cmds + i) + 1));
    strcpy(compile->command, compile_cmds + i);
    compile = compile->next;
    i++;
  }
  i = 0;
  while (test_cmds + i != NULL) {
    test = malloc(sizeof(Command));
    test->command = malloc(strlen((test_cmds + i) + 1));
    strcpy(test->command, test_cmds + i);
    test = test->next;
    i++;
  }
  return commands;
}

Upvotes: 1

Views: 458

Answers (1)

AndersK
AndersK

Reputation: 36102

You should change the arguments to accept multiple commands e.g.

Commands read_commands(const char** compile_cmds, const char** test_cmds)

then you call it with:

char* compileCmds[] = { "abc", NULL };
char* testCmds[] = { "abc", NULL };

Commands c = read_commands(compileCmds,testCmds);

Meanwhile in your function to get all "compile" commands:

Commands commands = { NULL, NULL };
...

Command* last = NULL;
for (i = 0; compile_cmds[i] != NULL; ++i)
{
  compile = malloc(sizeof(Command));

  // check if we have added before, or if it is the first compile command
  if (last!=NULL)
  {
    // last command, so append
    last->next = compile;
  }
  else
  {
    // first command, set as first
    commands->compile = compile;
  }

  // add the payload
  compile->command = strdup(compile_cmds[i]);
  compile->next = NULL;

  // keep track of last to easy append
  last = compile;
}

...

you may have noticed you have duplicate code in your function, so an idea would be to create a function read_commands for one of the compile or tests at a time and call it twice instead.

e.g.

 read_commands(Command** cmd, const char** cmds);
 ...
 Commands c = {NULL,NULL};
 read_commands(&c.compile, compileCmds);
 read_commands(&c.test, testCmds);

the extra * is for to be able to change what the pointer points to

 read_commands(Command** pc, const char* cmds)
 {
    ... 
    *pc = malloc(sizeof(Command));
    ...
 }

Upvotes: 1

Related Questions