Segmentation fault at the end of a *char array

Question

I'm new to using C and I'm running into a problem. I want to use the following code to split-up a string input from the linux command line (something like date | ./date_split) into an array, which I will be later accessing and modifying. The splitting works initially but I get a segmentation fault at the end. Can anyone explain what I am doing wrong?

int main()
{
   char *indate[10];
   int i= 1;
   char str[100][50];
   fgets(&str [0], 50, stdin);
   const char s[2] = " ";
   char *token;

  /* get the first token */
  token = strtok(str, s);
  indate[i] = malloc(strlen(token)+1);
  strcpy(indate[i], token);
  printf( "Array before: %s\n", indate[i]);     

 /* walk through other tokens */
 while( token != NULL )
{
  printf( "Token before: %s\n", token );

  token = strtok(NULL, s);
  indate[i] = malloc(strlen(token)+1);
  strcpy(indate[i], token);

printf( "Token2 After: %s\n", token );
printf( "Array2 After: %s\n", indate[i]);     
i++;   
}

    return(0);
}

Which gives terminal output:

Array before: Thu
Token before: Thu
Token2 After: 20
Array2 After: 20
Token before: 20
Token2 After: Oct
Array2 After: Oct
Token before: Oct
Token2 After: 11:37:56
Array2 After: 11:37:56
Token before: 11:37:56
Token2 After: EDT
Array2 After: EDT
Token before: EDT
Token2 After: 2016

Array2 After: 2016

Token before: 2016

Segmentation fault (core dumped)

`fgets(&str[0], 50, stdin);` looks wrong to me. You're passing a `char **` value instead of `char *`. Did you compile this code with warnings enabled? Try adding `-Wall` to the command line options. **Edit:** `token = strtok(str, s);` is also wrong; `str` should be `char *`, not `char **`. This would be a good time to start learning how to use a [debugger](https://en.wikipedia.org/wiki/GNU_Debugger). — r3mainer, Oct 20 '16 at 15:58
It does indeed give me warnings at fgets as you said, and I had no idea a debugger existed thanks for letting me know! I've figured it out how to get it to run without a segmentation fault, when I add an `if (token==Null) {break;}` before updating the array. But I still don't know why that works! — Meme Overlord, Oct 20 '16 at 16:03

score 2 · Accepted Answer · answered Oct 20 '16 at 17:01

Issues with your program: your index into the date components, i, starts at 1 instead of 0 (is a terrible variable name for this purpose) and isn't updated consistently (first entry gets overwritten); the str() array is a total mess allocation-wise (e.g. two dimensions, only one used); you assume the first strtok() succeeds but that might not be the case with bad input; you don't test if the subsequent strtok() calls succeed until after you've already used the result; you make no attempt to free() the memory you malloc() and even lose track of some.

Below is a rework of your original code that also adds some error checking and other niceties:

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

#define MAXIMUM_TOKENS 16
#define MAXIMUM_TOKEN_LENGTH 32

const char *separator = " ";

int main()
{
    char string[(MAXIMUM_TOKEN_LENGTH + strlen(separator)) * MAXIMUM_TOKENS + 1]; // estimate

    char *result = fgets(string, sizeof(string), stdin);

    if (result == NULL)
    {
        fprintf(stderr, "An appropriate error message goes here.\n");
        return EXIT_FAILURE;
    }

    /* strip final newline (\n) if present */
    size_t last_index = strlen(string) - 1;

    if (string[last_index] == '\n')
    {
        string[last_index] = '\0';
    }

    /* get the first token */
    char *token = strtok(string, separator);

    char *date_parts[MAXIMUM_TOKENS];
    int date_parts_index = 0;

    /* walk through other tokens */
    while (token != NULL)
    {
        date_parts[date_parts_index++] = strdup(token);
        token = strtok(NULL, separator);
    }

    /* print the tokens and free the strdup/malloc memory */
    for (int i = 0; i < date_parts_index; i++)
    {
        (void) puts(date_parts[i]);
        free(date_parts[i]);
    }

    return EXIT_SUCCESS;
}

USAGE

% date | ./a.out
Thu
Oct
20
09:58:00
PDT
2016
%

Although this is an appropriate use of strtok(), be wary of it. It is an artifact of an earlier age and should be avoided in favor of safer, modern library functions like strsep() and strtok_r().

Wow, that is infinitely better than what I had written, thank you so much! I'll take your suggestions to heart and modify what I had written accordingly — Meme Overlord, Oct 20 '16 at 17:18

score 1 · Answer 2 · edited May 23 '17 at 10:27

1

From strtok() manual

RETURN VALUE The strtok() and strtok_r() functions return a pointer to the next token, or NULL if there are no more tokens.

Once there are no more tokens, token contains a NULL pointer, and therefore strlen(token) seg faults. See strlen not checking for NULL

edited May 23 '17 at 10:27

Community

1
1

answered Oct 20 '16 at 16:18

Luis de Arquer

377
2
8

Segmentation fault at the end of a *char array

2 Answers2