1

I use select() to know when I must read from stdin. I call this function:

void
CLI()
{
    char *line=(char*)malloc(sizeof(char)*32);
    char *cmd=(char*)malloc(sizeof(char)*16);
    char *arg1=(char*)malloc(sizeof(char)*8);
    char *arg2=(char*)malloc(sizeof(char)*8);

    while(fgets(line, sizeof(line), stdin) != NULL)
    {
        cmd=strtok(line," \n\r\t");
        arg1=strtok(NULL," \n\r\t");
        arg2=strtok(NULL," \n\r\t");

        if(cmd==NULL) break;

        printf("cmd=%s, arg1=%s, arg2=%s",cmd,arg1,arg2);
    }
    free(line);
    free(cmd);
    free(arg1);
    free(arg2);
}

Example of input: #set PAR 0

What I get:

*** Error in './myprogram': double free or corruption (fasttop): 0x0000000001cc70f0 ***

What am I doing wrong?

Yu Hao
  • 119,891
  • 44
  • 235
  • 294
anat0lius
  • 2,145
  • 6
  • 33
  • 60

2 Answers2

3

The problems here are

  1. fgets(line, sizeof(line), stdin, no that's not what you want. sizeof() returns the size of the datatype, not the area pointed by the pointer. You have to supply the amount of memory available for use through that pointer, something like

    while(fgets(line, 32 , stdin) != NULL) //sizeof(char) == 1, can be ommitted

  2. After allocating by doing malloc() to a pointer, using assignment = leads to memory leak. Use strcpy() instead to copy the token returned by strtok() to the required pointer.

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • Tryed with strcpy, but I get `segmentation fault`. So much with malloc as without. Btw, fgets requiers line to be initialized. – anat0lius Apr 07 '15 at 11:56
  • @LiLou_ Can you use `gdb` and give us some more info? Which line getting segfault? – Sourav Ghosh Apr 07 '15 at 11:57
  • is at fgets. I init line with NULL. – anat0lius Apr 07 '15 at 11:58
  • 1
    @LiLou_ I think there is a confusion. you have to allocate memory for all, as you were doing in your code. What i asked you is to 1) follow my point 1. 2 ) instead of `cmd=strtok(line," \n\r\t");`, take a temporary pointer and use like `char *tmp; tmp=strtok(line," \n\r\t"); strcpy(cmd, tmp);`. Hope you got the idea. – Sourav Ghosh Apr 07 '15 at 12:05
1

Remove

char *cmd=(char*)malloc(sizeof(char)*16);
char *arg1=(char*)malloc(sizeof(char)*8);
char *arg2=(char*)malloc(sizeof(char)*8);

Why? Because the memory you have malloced is lost here:

cmd=strtok(line," \n\r\t");
arg1=strtok(NULL," \n\r\t");
arg2=strtok(NULL," \n\r\t");

strtok returns a char*. You change the location that the pointers point to. This leads to a memory leak as you haven't freeed the allocated memory. You don't need

free(cmd);
free(arg1);
free(arg2);

once you have removed the three calls to malloc as mentioned above.

Also, Don't cast the result of malloc and family. Do note that sizeof(char) is 1 as multiplying it with another number in the malloc isn't needed.

Community
  • 1
  • 1
Spikatrix
  • 20,225
  • 7
  • 37
  • 83