Search code examples
csegmentation-faultstrtok

Seg fault when using strtok


I am trying to read each number from an input that looks like this:

179    2358 5197    867 
5541    172 4294    1397
2637    136 3222    591 

... and get the min/max of each row going line by line and then getting each number in the line using strtok, but I get a seg fault error in the second while loop.

The code:

#include "header.h"
#include <string.h>

int main(void) {
  int8_t* line = malloc(sizeof(int8_t) * 75);
  int8_t* temp;

  int32_t minim, maxim;
  int32_t current;

  stdin = fopen("stdin", "r");
  while (fgets(line, 75 * sizeof(int8_t), stdin)) {
    printf("%s", line);
    temp = strtok(line," \n\t");
    maxim = minim = atoi(temp);

    while (temp != NULL) {
      current = atoi(temp);
      temp = strtok(NULL, " \n\t");

      if (current < minim)
        minim = current;
      if (current > maxim)
        maxim = current;

    } 

    printf("\nMin and max: %d %d\n", minim, maxim);
  }

  printf("\n");
  return 0;
}

The header:

#ifndef HEADER_H
# define HEADER_H

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

  typedef                   char  int8_t;
  typedef   signed     short int int16_t;
  typedef   signed           int int32_t;
  typedef   signed long long int int64_t;

  typedef unsigned          char  uint8_t;
  typedef unsigned     short int uint16_t;
  typedef unsigned           int uint32_t;
  typedef unsigned long long int uint64_t;

#endif

I just don't get what it might be wrong. Thanks.


Solution

  • OK, so...

    1. Your <header.h> breaks on my compiler, as it redefines the identifiers int8_t et al. reserved by the standard. (Not quite correct of the compiler to complain, as the relevant headers were not included, but there you are.) If you define your own types, use your own identifiers. Or include <stdint.h> for the "real" intX_t types (yes I know a certain "big" compiler doesn't have that header).

    2. The simplest thing, of course, is to use char * for strings instead of int8_t *. (The signed-ness of char is implementation-defined, and anyway a good compiler complains rather loudly about the many implicit casts not using char * involves.)

    3. You use the exact-width type int32_t for your minim and maxim, but neglect to use the proper printf() width specifiers to go with that (PRId32, specified in <inttypes.h>). As I cannot see any specific need for exact-width types, I replaced them with plain int (for which a mere %d suffices).

    4. Once you've done that, remove mention of sizeof( char ), as that is by definition 1.

    5. Do check return values of functions. fopen() can fail. Make sure it didn't.

    6. Don't "reuse" the stdin file descriptor. It's just confusing all around. Whether your stdin comes from the terminal or a file or a pipe is decided by how you call your executable. If you open a file, use a FILE * handle_name, not stdin. And besides, stdin is an open file descriptor (namely your program's standard input), so you would have to use freopen(), not fopen(), to have it properly receiving from a different source. The simplest thing is to actually use stdin as-is (which also gives you the greatest flexibility in using your program).

    7. Do release resources you acquired. free() memory. fclose() files. Many operating systems protect you from this kind of negligence and clean up after you. Some don't.

    And once you've fixed those problems you sneezed at in comments...

    #include <stdlib.h>                           // <-- added
    #include <stdio.h>                            // <-- added
    #include <string.h>
    
    int main() {
      char* line = malloc(75);                    // <-- turned to char *
      char* temp;                                 // <-- turned to char *
    
      int minim, maxim;                           // <-- turned to int
      int current;                                // <-- turned to int
    
      while (fgets(line, 75, stdin)) {            // <-- using stdin directly
        printf("%s", line);
        temp = strtok(line," \n\t");
        maxim = minim = atoi(temp);
    
        while (temp != NULL) {
          current = atoi(temp);
          temp = strtok(NULL, " \n\t");
    
          if (current < minim)
            minim = current;
          if (current > maxim)
            maxim = current;
    
        }
    
        printf("\nMin and max: %d %d\n", minim, maxim);
      }
    
      printf("\n");
      free( line );                                // <-- releasing memory
      return 0;
    }
    

    ...your program works:

    ./testme.exe < file.dat                      # feeding file.dat to stdin
    179    2358 5197    867
    
    Min and max: 179 5197
    5541    172 4294    1397
    
    Min and max: 172 5541
    2637    136 3222    591
    
    Min and max: 136 3222
    

    Lesson here: Code cleanly. Enable strictest compiler warnings, and heed them.

    As for overwriting stdin with the results of fopen() as you did, the standard states (emphasis mine):

    229) [...] [stderr, stdin, or stdout] need not be modifiable lvalues to which the value returned by the fopen function may be assigned.