Segmentation fault when using array of strings in C

Question

I am new in C and all this thing of strings is pretty confusing.

The objective of this part of the program is to split a string into multiple strings, in this way I can process the words separately.

 int SplitString(char * str, char * pieces[]) {
  int i=1;

  if ((pieces[0]=strtok(str," \n\t"))==NULL){
    return 0;
  }

  while ((pieces[i]=strtok(str," \n\t"))!=NULL) {
    i++;
  }

  return i;
}

/*****************************************************************************/
void CommandPros(char *str) {
  char *pieces[100];
  int numW = 0;

  for (int i = 0; i < 100; i++) {
    pieces[i] = (char *) malloc (100*sizeof(char));
  }

  numW = SplitString(str, pieces);

}

/*****************************************************************************/
void ReadEnter(char * str) {
  fgets(str, 100, stdin);
}

/*****************************************************************************/
 int main(){
  int fin = 0;
  char  * strCommand;
  strCommand = (char *) malloc (100*sizeof(char));

  while (!fin) {
    ReadEnter(strCommand); 
    CommandPros(strCommand);
  }

  return 0;
}

But when I execute the program this message appear:

Segmentation fault: 11

that come later, the program is incomplete my problem is the segmentation fault in the function SplitString. — Rodrigo Diaz
– Rodrigo Diaz, Commented Sep 18, 2019 at 15:42
What is PrintPrompt? And please provide a sample input that triggrs the promlem. Read this: How to Ask and this: minimal reproducible example — Jabberwocky
– Jabberwocky, Commented Sep 18, 2019 at 15:42
the problem is when the function is invoked SplitString. it never gets inside the function — Rodrigo Diaz
– Rodrigo Diaz, Commented Sep 18, 2019 at 15:44

mangusta · Accepted Answer · 2019-09-18 15:45:59Z

2

You should use NULL as a first argument starting from second call to strtok

Replace
while ((pieces[i]=strtok(str," \n\t"))!=NULL)

with
while ((pieces[i]=strtok(NULL," \n\t"))!=NULL)

answered Sep 18, 2019 at 15:45

mangusta

3,5746 gold badges30 silver badges54 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Rodrigo Diaz Over a year ago

Thanks!! very useful.

Jabberwocky · Accepted Answer · 2019-09-18 15:57:18Z

1

Your SplitString function is overly complicated and wrong.

You want this:

int SplitString(char * str, char * pieces[]) {
  int i = 0;

  char *token = strtok(str, " \n\t");

  while (token != NULL)              // while there are tokens
  {
    strcpy(pieces[i++], token);      // copy the string, not the pointer
    token = strtok(NULL, " \n\t");   // use NULL here, read the
                                     // documentation of strtok
  }

  return i;
}

Not directly related to your problem:

The CommandPros function is quite awkward: you allocate memory for 100 strings each of which has room for 99 characters (+ the NUL terminator), which is in most cases too much.

You should allocate only the memory you need. The overall design of your code looks wrong.

edited Sep 18, 2019 at 15:57

answered Sep 18, 2019 at 15:52

Jabberwocky

51.3k18 gold badges71 silver badges127 bronze badges

Comments

Neena · Accepted Answer · 2019-09-18 16:05:03Z

1

You have to modify the second strtok() in SplitString to below while ((pieces[i]=strtok(NULL," \n\t"))!=NULL)

The first call to strtok must pass the C string to be tokenize, and subsequent calls must specify NULL as the first argument, which tells the function to continue tokenizing the string you passed in first.

answered Sep 18, 2019 at 16:05

Neena

514 bronze badges

Comments

user3629249 · Accepted Answer · 2019-09-19 18:07:19Z

It is always best to check the returned value from a call to a C library function to assure the operation was successful. In this case, malloc() and fgets()

regarding:

/*****************************************************************************/
void ReadEnter(char * str) {
    fgets(str, 100, stdin);
}

int main(){
    int fin = 0;
    char  * strCommand;
    strCommand = (char *) malloc (100*sizeof(char));

    while (!fin) {
       ReadEnter(strCommand); 
       CommandPros(strCommand);
   }

There needs to be some 'consistent' method of terminating the loop in main(). Paying attention to the returned value from the call to fgets() will do that very nicely. Suggest:

int main( void )
{
    char  strCommand[ 100 ];

    while ( fgets( strCommand, 100, stdin ) )
    { 
       CommandPros(strCommand);
    }
}

also, the function: SplitString() allocates another 100 parts of dynamic memory from the heap every time it is called. BUT none of the code ever passes each of those (100 * number of lines in input) to free(). This results in a massive memory leak

The function: SplitString() only sets a pointer to each of the areas in strCommand that were found via strtok() and the expectation seems to be that each of the 100 allocated memory areas is expected to contain the actual contents of the string found by strtok() Suggest:

int SplitString( char * str, char * pieces[], int maxPieces ) 
{
    int i = 0;

    char *token = strtok(str, " \n\t");
    while ( i < maxPieces && token != NULL )              
    {
         strcpy(pieces[i++], token);      
         token = strtok(NULL, " \n\t");                  
    }

    return i;
}

In the function: CommandPros() the local variable: num is set to the number of words found in the current str but is never used for anything. The compiler will output a warning message about this problem.

Also, the posted code contains a 'magic' number 100. 'magic' numbers are numbers with no basis and they make the code much more difficult to understand, debug, etc. Suggest using a enum statement or #define statement to give that 'magic' number a meaningful name then use that meaningful name throughout the code.

Collectives™ on Stack Overflow

Segmentation fault when using array of strings in C

4 Answers 4

1 Comment

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related