0

I'm writing a C program that will open a file, write to it, and then read what was written. I can open, write, and close the file, but I can't read the lines and parse them correctly.

I have read many other blogs and sites, but none fully address what I'm trying to do. I've tried adapting their general solutions, but I never get the behavior I want. I have run this code with fgets(), gets(), strtok(), and scanf(), and fscanf(). I used strtok_r() as it was recommended as best practice. I used gets() and scanf() as experiments to see what their output would be, as opposed to fgets() and fscanf().

What I want to do:

  1. get first line // fist line is a string of space delimited ints "1 2 3 4 5"
  2. parse this line, convert each char number into a integer
  3. store this into an array.
  4. get the next line and repeat until EOF

Can someone please tell me what I'm missing and what functions would be considered best practice?

Thanks

My code:

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

int main(){
  FILE * file;

  // read data from customer.txt
  char lines[30];
  file = fopen("data.txt", "r"); 
  // data.txt currently holds five lines
  // 1 1 1 1 1 
  // 2 2 2 2 2
  // 3 3 3 3 3
  // 4 4 4 4 4 
  // 5 5 5 5 5

  char *number;
  char *next = lines;


  int s = 0;
  int t = 0;
  int num;
  int prams[30][30];

  while(fgets(lines, 30, file)){
        char *from = next;

    while((number = strtok_r(from, " ", &next)) != NULL){
        int i = atoi(number);
        prams[t][s] = i;
        printf("this is prams[%d][%d]: %d\n", t, s, prams[t][s]);

        s++;
        from = NULL;               
    }

    t++;
  }

  fclose(file);
}// main

expected output:

this is prams[0][0]: 1
...
this is prams[4][4]: 5

Actual output:

this is prams[0][0]: 1
this is prams[0][1]: 1
this is prams[0][2]: 1
this is prams[0][3]: 1
this is prams[0][4]: 1
program ends

Rice Man
  • 415
  • 1
  • 5
  • 16
  • 4
    [Why is “while (!feof(file))” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) – Eugene Sh. Mar 25 '19 at 16:26
  • @EugeneSh. Is that just the standard comment or in this case actually the explanation for the problem? – Yunnosch Mar 25 '19 at 16:29
  • See [How to use `sscanf()` in loops](http://stackoverflow.com/questions/3975236/how-to-use-sscanf-in-loops) for how to parse the line with `sscanf()`. Use `while (fgets(lines, 30, file))` for the outer loop control; don't use `feof()` except (perhaps) after a loop terminates to distinguish between EOF and I/O error. – Jonathan Leffler Mar 25 '19 at 16:30
  • @Yunnosch the answer to that question ought to help the author of this one – Tim Randall Mar 25 '19 at 16:31
  • @ EugeneSh I don't know. From my understanding there is a difference between '\n', '\0', and EOF. I understand "while (!feof(file)" to mean: keep looping until you reach the EOF indicator which maybe \0 or some unique EOF char. However, it always ends after only parsing the first line. – Rice Man Mar 25 '19 at 16:33
  • 1
    @Yunnosch Kind of standard. But once I see one problem, I generate kind of "reading stopped because of previous errors" state :) – Eugene Sh. Mar 25 '19 at 16:35
  • @ Tim Randall, the output here is only a sanity check so that I will know what is in the 2D int array prams[][]. The expected print output should be 25 lines indicating what int is being held in each location of the prams array. – Rice Man Mar 25 '19 at 16:38
  • @RiceMan your still wrongly use `strtok_r` and you missed to reset the column number when you start a new line (with potential catastrophic effects), see my answer (with other remarks) – bruno Mar 25 '19 at 18:11

3 Answers3

1

The direct main problem is that you keep telling strtok_r() to start at the beginning of the string, so it keeps on returning the same value. You need to set the first parameter to strtok_r() to NULL so that it continues where it left off:

char *from = next;
while ((number = strtok_r(from, " ", &next)) != NULL)
{
    int i = atoi(number);
    prams[t][s] = i;
    printf("this is prams[%d][%d]: %d\n", t, s, prams[t][s]);
    s++;
    from = NULL;               
}

There are those who'd argue in favour of strtol() over atoi(); there is some justice on their side, but probably not enough to matter.

See also How to use sscanf() in loops? for how to parse the line with sscanf().

Use:

while (fgets(lines, 30, file))

for the outer loop control; don't use feof() except (perhaps) after a loop terminates to distinguish between EOF and I/O error. (A few years back, I checked through multiple hundreds of C source files of mine and found less than half a dozen uses of eof(), all in error checking code and none in loop controls. You really won't need to use it very often at all.)

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • I updated my code and the code I posted to reflect those changes, however the behavior is the same and the program ends after one outer loop iteration. Any ideas? – Rice Man Mar 25 '19 at 17:01
  • Thank you for the insight on from = NULL and feof(). – Rice Man Mar 25 '19 at 18:18
1

The main problems are :

  • you never reset s to 0, so the column always increase rather than to be from 0 to 4 (if 5 numbers per line), so you do not write on the expected entries in the array from the second line and you have a risk to write out of the array with an undefined behavior (like a segmentation fault)
  • check you do not read too much columns and lines (30 in your code), else you can write out of the array with an undefined behavior (like a segmentation fault)
  • you use wrongly strtok_r, the first parameter must be not null only the first time you parse a line (before your edit)
  • doing number = strtok_r(from, " ", &next) next is modified by strtok_r while it is used to initialize from for the next line, so the second line will not be read correctly and your execution is only :

this is prams[0][0]: 11
this is prams[0][1]: 12
this is prams[0][2]: 13
this is prams[0][3]: 14
this is prams[0][4]: 15
this is prams[3][5]: 0

with data.txt containing :

11 12 13 14 15
21 22 23 24 25
31 32 33 34 35
41 42 43 44 45
51 52 53 54 55

(also look at the indexes [3][5] because you missed to reset s )

Additional remarks :

  • check fopen success
  • initialize prams or memorize how much columns there are on the first line and check it is always the same number of column on the next lines, also memorize how much lines of course, else you don't know later where are the read numbers in the array
  • atoi does not indicates if you read a number or not

A proposal to take these remarks into account is (I initialize the array with 0 without making assumption on the number of numbers per line) :

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

#define LINELENGTH 30
#define SIZE 30

int main(){
  // read data from customer.txt
  char lines[LINELENGTH];
  FILE * file = fopen("data.txt", "r"); 

  if (file == NULL) {
    fprintf(stderr, "cannot read data.txt");
    return -1;
  }

  // data.txt currently holds five lines
  // 1 1 1 1 1 
  // 2 2 2 2 2
  // 3 3 3 3 3
  // 4 4 4 4 4 
  // 5 5 5 5 5

  int t = 0;
  int prams[SIZE][SIZE] = { 0 };

  while (fgets(lines, LINELENGTH, file)) {
    char * number;
    char * str = lines;
    int s = 0;

    while ((number = strtok(str, " \n")) != NULL) {
      char c;
      int i;

      if (sscanf(number, "%d%c", &i, &c) != 1) {
        fprintf(stderr, "invalid number '%s'\n", number);
        return -1;
      }
      prams[t][s] = i;
      printf("this is prams[%d][%d]: %d\n", t, s, prams[t][s]);
      str = NULL;
      if (++s == SIZE)
        break;
    }

    if (++t == SIZE)
      break;
  }

  fclose(file);
}// main

I use sscanf(number, "%d%c", &i, &c) != 1 to easily detect if a number and only a number is read or not, note I added \n is the separators for strtok

Compilation and execution :

pi@raspberrypi:/tmp $ !g
gcc -pedantic -Wall -Wextra l.c
pi@raspberrypi:/tmp $ cat data.txt 
11 12 13 14 15
21 22 23 24 25
31 32 33 34 35
41 42 43 44 45 
51 52 53 54 55
pi@raspberrypi:/tmp $ ./a.out
this is prams[0][0]: 11
this is prams[0][1]: 12
this is prams[0][2]: 13
this is prams[0][3]: 14
this is prams[0][4]: 15
this is prams[1][0]: 21
this is prams[1][1]: 22
this is prams[1][2]: 23
this is prams[1][3]: 24
this is prams[1][4]: 25
this is prams[2][0]: 31
this is prams[2][1]: 32
this is prams[2][2]: 33
this is prams[2][3]: 34
this is prams[2][4]: 35
this is prams[3][0]: 41
this is prams[3][1]: 42
this is prams[3][2]: 43
this is prams[3][3]: 44
this is prams[3][4]: 45
this is prams[4][0]: 51
this is prams[4][1]: 52
this is prams[4][2]: 53
this is prams[4][3]: 54
this is prams[4][4]: 55
bruno
  • 32,421
  • 7
  • 25
  • 37
  • You are right, thank you very much! I adjusted my code to reflect the changes you made and it behaved correctly. The two problems were: I didn't reset "s" correctly and strtok() was not formatted correctly after the first iteration. – Rice Man Mar 25 '19 at 18:40
  • @RiceMan warning the other remarks have also to be taken into account ;-) – bruno Mar 25 '19 at 18:44
0

If you want to parse whitespace-delimited text, then scanf and friends are your best bet. If, however, you want to treat newlines specially and NOT as whitespace, then you need the fgets+sscanf loop:

#define ROWS 30
#define COLS 30
#define MAXLINE 512
int prams[ROWS][COLS];
int row, col, len;
char buffer[MAXLINE], *p;

row = 0;
while (row < ROWS && fgets(buffer, MAXLINE, stdin)) {
    col = 0;
    p = buffer;
    while (col < COLS && sscanf(p, "%d %n", &prams[row][col], &len) > 0) {
        p += len;
        ++col; }
    if (*p) {
        /* extra stuff on the end of the line -- error? */ }
    ++row; }

Note ALSO checking the bounds to make sure the fixed-size array bounds are not exceeded.

Chris Dodd
  • 119,907
  • 13
  • 134
  • 226
  • Chris, thank you for this example. I was unable to get it to work with the current code I've written using a text file. However, your code style is very succinct and elegant and I plan on incorporating it. – Rice Man Mar 25 '19 at 18:48