malloc

[powerboy]

I've started messing around trying to code a mud just for the heck of it, but I'm having a problem with malloc. It exits the program with a 'Segmentation fault' error.

I'm compiling with gcc version 3 and running on RedHat 9.0

I've trimmed the code to illustrate the part that's causing the problem:

char cmd[MAX_CMD_LEN];
char cmd_cpy[MAX_CMD_LEN];
char *cmd_val;

  while("true")
  {    
     /* Allocate the exact amount of memory we need */
     cmd_val = (char *) malloc(MAX_CMD_LEN);

     /* get the command from the user */
     fgets( cmd, MAX_CMD_LEN, stdin );

     if (cmd_val == NULL)
     {
        printf("Couldn't allocate any memory");
        return;
     }

     /* make a copy for tokenising */
     strcpy(cmd_cpy, cmd);

     cmd_val = strtok(cmd_cpy, " ");  //first token of cmd_cpy

     /* free the memory otherwise this loop will
        eventually cause the program to crash */
     free(cmd_val);
     cmd_val = NULL;
    }
  }

Any help would be very much appreciated.

--
powerboy

[Shogun]

please use code tags.

[powerboy]

Ah sorry about that.

[kurz7]

i think u malloc'ed the wrong value, instead of mallocing the array u just malloc'ed the hash defined max_cmd_len

[kurz7]

i think u malloc'ed the wrong value, instead of mallocing the array u just malloc'ed the hash defined max_cmd_len

[Wledge]

The problem is here

cmd_val = strtok(cmd_cpy, " ");  //first token of cmd_cpy

You must pass strtok a string allocated on the heap (not stack) as first parameter.

[Prelude]

>You must pass strtok a string allocated on the heap (not stack) as first parameter.
Since when?

The real problem is that you allocate memory to cmd_val and then later assign the return value of strtok to that same variable. This is a memory leak and in this case a greivous error as you then attempt to free cmd_val. Attempting to free memory not returned by malloc and friends or NULL results in undefined behavior (or a seg fault in your case). cmd_val was no longer pointing to memory returned by malloc when you assigned the address of a section of cmd_cpy (a static array) to it.

[powerboy]

Thanks for the replies so far, but no luck yet. I've tried changing cmd_cpy to be dynamic, but I'm not sure if I'm on the right track:

  char cmd[MAX_CMD_LEN];
  char *cmd_cpy;
  char *cmd_val;
  char *cmd_arg;

  while("true")
  {    
    fgets( cmd, MAX_CMD_LEN, stdin );     
    cmd_cpy = (char *) malloc(MAX_CMD_LEN+1);
    cmd_val = (char *) malloc(MAX_CMD_LEN+1);
    cmd_arg = (char *) malloc(MAX_CMD_LEN+1);

    if (cmd_cpy == NULL || cmd_val == NULL || cmd_arg == NULL)
    {
      printf("Couldn't allocate any memory");
      return;
    }
    strcpy(cmd_cpy,(char *)cmd);
    cmd_val = (char *)strtok(cmd_cpy, " ");
    cmd_arg = (char *)strtok(NULL, " ");
    free(cmd_cpy);
    free(cmd_val);
    free(cmd_arg);  
    cmd_cpy = NULL;
    cmd_val = NULL;
    cmd_arg = NULL;    
  }

[Prelude]

>I've tried changing cmd_cpy to be dynamic, but I'm not sure if I'm on the right track
You're missing the point completely. You don't need dynamic allocation since you already have static arrays to work with:

while("true")
{
    /* get the command from the user */
    fgets( cmd, MAX_CMD_LEN, stdin );

    /* make a copy for tokenising */
    strcpy(cmd_cpy, cmd);
        
    cmd_val = strtok(cmd_cpy, " ");
}

[powerboy]

Thankyou again, I really appreciate your help

Okay I've got this now:

  char cmd[MAX_CMD_LEN];
  char cmd_cpy[MAX_CMD_LEN];
  char *cmd_val;
  char *cmd_arg;

  while("true")
  {    
    fgets( cmd, MAX_CMD_LEN, stdin );
    strcpy(cmd_cpy,cmd);

    cmd_val = strtok(cmd_cpy, " ");
    cmd_arg = strtok(NULL, " ");
  }

And I've got another problem; if I enter 'blah' then cmd_val has a new line character added to it; if I enter 'blah1 blah2' then cmd_arg has the newline character added to it, and cmd_val doesn't. Is there a way I can force either both to have a newline, or preferably neither of them?

[Prelude]

>Is there a way I can force either both to have a newline, or preferably neither of them?
The reason is that fgets places its delimiting character in the buffer. You can remove it with something similar to this:

while("true")
{
    char *newline;

    fgets( cmd, MAX_CMD_LEN, stdin );

    if ((newline = strrchr(cmd, '\n')) != NULL)
        *newline = '\0';

    strcpy(cmd_cpy,cmd);
        
    cmd_val = strtok(cmd_cpy, " ");
    cmd_arg = strtok(NULL, " ");
}

[powerboy]

Works a treat

So far things are going well, but I'll be back I'm sure...

Thanks for the help.