CODEWITHSUNDEEP
carraysfile
user7056422
user7056422

Reputation:

Split a string that I read from a file

I have a file like this

GET /index.html k
GET /docencia.html k
GET /ejemplo.html k

and I want to read it line by line and split it up with this delimiter " " but is giving me this error: segmentation fault(core dumped) and I don't know what to try.

This is my code:

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

int main(int argc, char *argv[]) {
    char ordenes[150];
    char *orden[3];
    char *token;
    int tok;

    FILE *fp;
    fp = fopen(argv[1], "r");
    if (fp == NULL) {
        printf("File error");
        exit(1);
    }

    while (feof(fp) == 0) {
        fgets(ordenes, sizeof(ordenes), fp);
        printf("%s \n", ordenes);
        token = strtok(ordenes, " ");
        tok = 0;
        while (token != NULL) {
            orden[tok] = strdup(token);
            tok++;
            token = strtok(NULL, " ");
        }

        printf("\n%s\n", orden[0]);
        printf("\n%s\n", orden[1]);
        printf("\n%s\n", orden[2]);
    }
    fclose(fp);
}

The error shows when I call the first strdup. If I try to print the token just after I call the first strtok, it fails too (the same segmentation fault core dumped) so I guess the problem is with the strtok.

Upvotes: 1

Views: 129

Answers (5)

chqrlie
chqrlie

Reputation: 144770

There are multiple problems in your code:

  • As alinsoar diagnosed with a sharp eye, you do not include <string.h>. strtok is not defined, the compiler must assume it returns an int, which it does not, and this int is silently converted to a char *. The code generated invokes undefined behavior and will most likely crash on 64-bit targets. You should compile with all warnings enabled to let the compiler help avoid this kind of silly mistake. gcc -Wall -W or clang -Weverything...

  • You do not check if command line arguments have been passed to your program before calling fp = fopen(argv[1], "r");. If no arguments are passed, argv[1] is a null pointer.

  • while (feof(fp) == 0) is incorrect, read Why is “while ( !feof (file) )” always wrong? . You should instead write while (fgets(ordenes, sizeof(ordenes), fp)) {...

  • You do not check if tok < 3 before storing token into the orden array. If the line has more than 3 fields, you will cause a buffer overflow.

  • You do not check if 3 tokens were indeed found before printing all 3 entries in orden. This too might invoke undefined behavior, especially if fgets() failed to read a line, which you do not check.

Here is an improved version:

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

int main(int argc, char *argv[]) {
    char ordenes[150];
    char *orden[3];
    char *token;
    int i, tok;

    if (argc < 2) {
        fprintf(stderr, "Missing command line argument\n");
        exit(1);
    }
    FILE *fp = fopen(argv[1], "r");
    if (fp == NULL) {
        fprintf(stderr, "Cannot open input file %s: %s\n", 
                argv[1], strerror(errno));
        exit(1);
    }

    while (fgets(ordenes, sizeof(ordenes), fp)) {
        printf("%s", ordenes);
        token = strtok(ordenes, " ");
        for (tok = 0; tok < 3 && token != NULL; tok++) {
            orden[tok] = strdup(token);
            token = strtok(NULL, " ");
        }
        for (i = 0; i < tok; i++) {
            printf("%s\n", orden[i]);
            free(orden[i]);
        }
        printf("\n");
    }
    fclose(fp);
    return 0;
}

Upvotes: 2

alinsoar
alinsoar

Reputation: 15793

You do not include <string.h>, so the compiler applies the default argument promotions on the signature of strtok, in particular it considers that strtok returns an int.

So the compiler will apply an operator of coercion from int to pointer to char at the assignment

token = strtok(ordenes, " ");

and this assignment will be compiled as

token = (int->char*) strtok(ordenes, " ");

Upvotes: 4

Stargateur
Stargateur

Reputation: 26727

You can do the following:

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

static void play_with_token(char *token, char const *delim)
{
  if (token == NULL)
    return;
  printf(" %s", token);
  play_with_token(strtok(NULL, delim), delim);
}

int main(int argc, char *argv[])
{
    if (argc != 2)
      return 1;
    FILE *fp = fopen(argv[1], "r");
    if (fp == NULL)
      return 1;

    char *line = NULL;
    size_t len = 0;
    ssize_t read;
    while ((read = getline(&line, &len, fp)) != -1) {
      printf("parsing line :");
      char const *delim = " ";
      play_with_token(strtok(line, delim), delim);
      printf("\n");
    }
   free(line);
   fclose(fp);
}

Upvotes: 0

Vlad from Moscow
Vlad from Moscow

Reputation: 310990

For starters you should change the condition in the outer loop statement the following way

while ( fgets(ordenes, sizeof(ordenes), fp) != NULL )

The condition in the inner loop should be written at least like

  while ( tok < 3 && token != NULL) {

The tokens should be outputted in a loop and the allocated memory must be freed. For example

  for ( int i = 0; i < tok; i++ )
  {
      printf("\n%s\n", orden[i]);
      free( orden[i]);
  }

Upvotes: 0

BLUEPIXY
BLUEPIXY

Reputation: 40145

try this:

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

int main(int argc, char *argv[]){
    char ordenes[150];
    char *orden[3];
    char *token;
    int tok;
    FILE *fp;

    fp = fopen (argv[1], "r");
    if(fp==NULL){
        printf("File error");
        exit(1);
    }
    while(fgets(ordenes, sizeof(ordenes), fp)){
        printf("%s\n",ordenes);
        token = strtok(ordenes, " ");
        tok = 0;
        while (token != NULL){
            orden[tok++] = strdup(token);
            token = strtok(NULL," ");
        }
        printf("\n%s\n",orden[0]);
        printf("\n%s\n",orden[1]);
        printf("\n%s\n",orden[2]);
        free(orden[0]);free(orden[1]);free(orden[2]);
    }
    fclose(fp);
}

Upvotes: -1

Related Questions