-3

I have to read a text file full of numbers and store each number into a string array then display it. Eventually I want to convert the numbers into integers but for now I just want to put them into a string array. My program displays only half the numbers along with a bunch of unwanted empty lines. Here's what I have so far:

void displayArray (char *a, int j) {
    for (int i = 0; i < j; i++) {
        printf("%c\n", a[i]);
    }
}

int main(int argc, const char *argv[]) {
    char temp[3];
    char str[26][3];
    int i = 0;

    // open file
    FILE *fp;
    fp = fopen("data.txt", "r");

    // access file
    while (!feof(fp)) {
        fgets(temp, 26, fp);
        //puts(temp);
        strcpy(str[i], temp);
        i++;
    }
    fclose(fp);

    displayArray(str, 26);

    return 0;
}

The data file is:

8
2
0
10
.5
3
7
3
7
12
12
12
12
1
10
.5
12
12 
12
12
7
3
7
3
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 2
    `char temp[3]; .... fgets(temp, 26, fp);` --> Code says `temp` is size 26 when it is only 3. Use `fgets(temp, sizeof temp, fp);` and check its return value rather than `feof()`. – chux - Reinstate Monica Oct 28 '17 at 03:33
  • You need to trim the `\n` character after `fgets`. [See Here](https://stackoverflow.com/questions/2693776/removing-trailing-newline-character-from-fgets-input)...and what @chux said. – Nik Oct 28 '17 at 03:39
  • You are passing `26` to `displayArray(str, 26)` but the lines in `data.txt` may be less than 26. Instead, you should pass `i` -> `displayArray(str, i)` – H.S. Oct 28 '17 at 04:05
  • You will want to look at [**Why is while ( !feof (file) ) always wrong?**](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). – David C. Rankin Oct 28 '17 at 06:24

2 Answers2

1

You have two primary problems, (1) the column width in str is not sufficient to hold 1, 0, \n, \0 that would be required by 10. While you will read 1, 0, \0, the '\n' will remain unread in your input stream. To correct the issue, increase the column width.

(never be stingy with buffer size, if you think you may read 3-4 chars, declare a buffer of 8 or so)

Second, you fail to remove the trailing '\n' read and included in str[x] by default. (all line-oriented functions, e.g. fgets and POSIX getline read and include the '\n' in the buffer they fill. Trimming a newline is simple. Get the length, check the last char if the last char in the buffer is '\n' by checking str[n][length-1]. If it is a '\n', the remove it by overwriting with the nul-terminating character, e.g.

    /* read each line up to MAXL lines */
    while (n < MAXL && fgets (str[n], MAXC, fp)) {
        size_t len = strlen (str[n]);   /* get length */
        long tmp;       /* tmp value for conversion */
        errno = 0;      /* reset errno before each conversion     */
        if (len && str[n][len-1] == '\n')   /* is last char '\n'? */
            str[n][--len] = 0;         /* overwrite with nul-char */
        else {           /* the string was too long, handle error */
            fprintf (stderr, "error: value at line %d too long.\n", n);
            return 1;
        }

(note: you also have the benefit of confirming if the line was too-long for the buffer if the last char was not '\n')

Next, you fail to validate the return of fgets. The read could fail and you still try and copy temp, e.g. (strcpy(str[i], temp);). You must validate ALL user input or you can have no confidence you are not simply processing garbage from that point on.

You can use strtol to convert your string to a long. Use a temporary long for the return from strtol, then, after checking errno to validate the conversion, you can validate the number returned is within the range of valid integers before assigning it to your integer array, e.g.

        tmp = strtol (str[n], NULL, BASE);  /* convert to long */
        if (errno) {                        /* validate conversion */
            fprintf (stderr, "error: conversion to int failed '%s'.\n",
                    str[n]);
            return 1;
        }   /* validate in integer range */
        if (tmp < INT_MIN || tmp > INT_MAX) {
            fprintf (stderr, "error arr[%ld] exceeds range of int.\n", 
                    tmp);
            return 1;
        }
        arr[n++] = tmp; /* assign converted & validated value to array */

Putting it altogether, you can read your file (or from stdin), store the values read as strings and convert the strings to integer values with something like the following:

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

enum { MAXC = 8, BASE = 10, MAXL = 26 };

int main (int argc, char **argv) {

    int n = 0,                      /* array index */
        arr[MAXL] = {0};            /* array of integers */
    char str[MAXL][MAXC] = {""};    /* array of strings */
    FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

    if (!fp) {  /* validate file open for reading */
        fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
        return 1;
    }

    /* read each line up to MAXL lines */
    while (n < MAXL && fgets (str[n], MAXC, fp)) {
        size_t len = strlen (str[n]);   /* get length */
        long tmp;       /* tmp value for conversion */
        errno = 0;      /* reset errno before each */
        if (len && str[n][len-1] == '\n')   /* is last char '\n'? */
            str[n][--len] = 0;         /* overwrite with nul-char */
        else {           /* the string was too long, handle error */
            fprintf (stderr, "error: value at line %d too long.\n", n);
            return 1;
        }
        tmp = strtol (str[n], NULL, BASE);  /* convert to long */
        if (errno) {                        /* validate conversion */
            fprintf (stderr, "error: conversion to int failed '%s'.\n",
                    str[n]);
            return 1;
        }   /* validate in integer range */
        if (tmp < INT_MIN || tmp > INT_MAX) {
            fprintf (stderr, "error arr[%ld] exceeds range of int.\n", 
                    tmp);
            return 1;
        }
        arr[n++] = tmp; /* assign converted & validated value to array */
    }
    if (fp != stdin) fclose (fp);   /* close file if not stdin */

    for (int i = 0; i < n; i++)     /* output both string and ints */
        printf ("str[%2d] : %-3s    arr[%2d] : %3d\n",
                i, str[i], i, arr[i]);

    return 0;
}

Example Use/Output

$ ./bin/cnvstrtoint < ../dat/numdata.txt
str[ 0] : 8      arr[ 0] :   8
str[ 1] : 2      arr[ 1] :   2
str[ 2] : 0      arr[ 2] :   0
str[ 3] : 10     arr[ 3] :  10
str[ 4] : .5     arr[ 4] :   0
str[ 5] : 3      arr[ 5] :   3
str[ 6] : 7      arr[ 6] :   7
str[ 7] : 3      arr[ 7] :   3
str[ 8] : 7      arr[ 8] :   7
str[ 9] : 12     arr[ 9] :  12
str[10] : 12     arr[10] :  12
str[11] : 12     arr[11] :  12
str[12] : 12     arr[12] :  12
str[13] : 1      arr[13] :   1
str[14] : 10     arr[14] :  10
str[15] : .5     arr[15] :   0
str[16] : 12     arr[16] :  12
str[17] : 12     arr[17] :  12
str[18] : 12     arr[18] :  12
str[19] : 12     arr[19] :  12
str[20] : 7      arr[20] :   7
str[21] : 3      arr[21] :   3
str[22] : 7      arr[22] :   7
str[23] : 3      arr[23] :   3

note: the integer conversion of .5 above results in 0, so if you need to store floating point values, you must declare the array to hold the converted values to double or float and use a floating-point conversion like strtod for the conversion.

Look things over and let me know if you have further questions.


Handling non-long integer Lines

As you have found, depending on your implementation, strtol may not accept and truncate .5 to zero, instead it may set errno. You have the flexibility to handle a failed conversion any way you choose. For example, if you want to assign 0 manually on a failed conversion and keep the index consistent with the number of lines read, you can warn on the failed conversion and manually assign 0, e.g.

    if (errno) {                        /* validate conversion */
        /* if you want to assing zero on a failed conversion, warn */
        fprintf (stderr, "warning: conversion to int failed '%s' - "
                "assigning 0.\n", str[n]);
        tmp = 0;
    }

(output is the same)

On the other hand, if you simply want to skip any values that are not converted by strtol, you can simply warn and continue (the line having already been read by fgets, continuing just advances to the next line in the file):

    if (errno) {                        /* validate conversion */
        /* if you want to skip the value entirely, just continue */
        fprintf (stderr, "warning: conversion to int failed '%s' - "
                "skipping.\n", str[n]);
        continue;
    }   /* validate in integer range */

Example Use/Output (skipping non-long integer values

$ ./bin/cnvstrtoint < ../dat/numdata.txt
str[ 0] : 8      arr[ 0] :   8
str[ 1] : 2      arr[ 1] :   2
str[ 2] : 0      arr[ 2] :   0
str[ 3] : 10     arr[ 3] :  10
str[ 4] : 3      arr[ 4] :   3
str[ 5] : 7      arr[ 5] :   7
str[ 6] : 3      arr[ 6] :   3
str[ 7] : 7      arr[ 7] :   7
str[ 8] : 12     arr[ 8] :  12
str[ 9] : 12     arr[ 9] :  12
str[10] : 12     arr[10] :  12
str[11] : 12     arr[11] :  12
str[12] : 1      arr[12] :   1
str[13] : 10     arr[13] :  10
str[14] : 12     arr[14] :  12
str[15] : 12     arr[15] :  12
str[16] : 12     arr[16] :  12
str[17] : 12     arr[17] :  12
str[18] : 7      arr[18] :   7
str[19] : 3      arr[19] :   3
str[20] : 7      arr[20] :   7
str[21] : 3      arr[21] :   3

Look things over a second time and let me know if you have further questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • It is not working for me. It keeps giving me error: conversion to int failed '%s'.\n cause of .5. or it closes my console for some reason. But i do understand a lot of it now. Thank you! I'm actually trying to do this without worrying about converting to integer I just want it in a string array. Also how do validate fgets exactly and how are you incrementing n? – Jack Johnson Oct 29 '17 at 03:53
  • That's a possibility. What OS? You see `.5` isn't an integer, and certain implementation will not simply truncate to `zero`. I'll drop an addition and show you how to handle it without exiting. – David C. Rankin Oct 29 '17 at 06:52
0

There are multiple problems in your code:

  • while(!feof(fp)) does not work: feof(fp) returns true after an attempt at reading the file has failed. Use fgets() and check the return value. NULL indicates an end of file or a read error.
  • char str[3] is too short to accommodate for a 2 digit number as the newline is also stored into the destination array. Make this array much larger.
  • your program would have undefined behavior if the file has more than 26 lines. You should either test the nuber of lines or reallocate the destination array.
  • the output loop has an incorrect prototype and implementation: it should be:

    void displayArray(char a[][3], int j) {
        for (int i = 0; i < j; i++) {
            printf("%s\n", a[i]);
        }
    }
    

Here is a naive approach to reading a string array of arbitrary length:

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

int main(int argc, char *argv[]) {
    char buf[100];
    char *array = NULL;
    size_t i, count = 0;
    FILE *fp = stdin;

    if (argc > 1) {
        fp = fopen(argv[1], "r");
        if (fp == NULL) {
            fprintf(stderr, "cannot open %s: %s\n", argv[1],
                    strerror(errno));
            return 1;
        }
    }

    while (fgets(buf, sizeof buf, fp)) {
        buf[strcspn(buf, "\n")] = '\0';  // strip the newline if any
        array = realloc(array, sizeof(*array) * (count + 2));
        array[count++] = strdup(buf);
    }

    if (fp != stdin)
        fclose(fp);

    for (i = 0; i < count; i++) {
        printf("%s\n", array[i]);
    }

    free(array);

    return 0;
}

Notes:

  • there is still a limit of 98 bytes for the maximum line length handled by this code.
  • memory allocation failure should be tested and reported.
  • if the goal is to read numbers, the conversion should probably take place in the reading loop, to avoid useless complications.
chqrlie
  • 131,814
  • 10
  • 121
  • 189