Problem with while loop when I read words from a file in C-CodePudding

I am trying to create a program which :

finds the word with largest length from a file
creates a dynamically allocated 2d array
stores the words from the file to array

https://gist.github.com/up1047388/363854cbe703a6f297ebb644c50d307f (the whole program)

the file: https://gist.github.com/up1047388/758574c484d916c4aeba106f293f185a

The problem is in this loop (I used printf as a hint to find the problem but the program does not print anything inside the problem):

That's I am trying to do in the loop is the following :

pass the character which has every line of the file in a string
when the program reads the '\n' from the file stores the word which is stored in the string to the array

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

int main()
{
    FILE *fp;

    fp = fopen("C:\\Users\\Docs\\lol.txt", "r");

    if (fp == NULL)
    {
        printf("the file is empty");
        exit(8);
    }

    int length[4];
    int ch1;
    int counter1 = 0, i = 0;

    while ((ch1 = getc(fp)) != EOF)
    {
        if (ch1 == '\n')
        {
            length[i] = counter1;
            i  ;
            counter1 = 0;
            continue;
        }
        counter1  ;
    }
    length[i] = counter1;

    fseek(fp, 0, SEEK_SET);

    int thelongest = length[0];

    for (i = 0; i < counter; i  )
    {
        if (length[i] >= thelongest)
        {
            thelongest = length[i];
        }
    }

    printf("\n the longest number is %d", thelongest);

    char **arr;

    **arr = (char **)malloc(sizeof(char *) * 4);

    int j = 0 i = 0;
    char str[thelongest];
    int ch2;
    while ((ch2 = getc(fp)) != EOF)
    { // printf("fvfdvdfvd");
        if (ch2 == '\n')
        {
            arr[j] = (char *)malloc(thelongest * sizeof(char));
            strcpy(arr[j], str);
            // printf("\n%c",str[i]);
            j  ;
            continue;
        }
        str[i] = ch2;
        i  ;
    }
    str[i] = ch2;
    strcpy(arr[j], str);

    return 0;
}

CodePudding user response：

There are multiple problems:

the diagnostic message is incorrect if fopen fails
you have undefined behavior if the file has more than 4 lines
**arr = (char **)malloc(sizeof(char *) * 4); is incorrect! you should write:
```
 arr = malloc(sizeof(*aff) * count);
```
char str[thelongest]; is not long enough to store a string of length thelongest: you need an extra byte for the null terminator.
arr[j] = (char *)malloc(thelongest * sizeof(char)); is too short for the longest lines and too large for other ones. You should use strdup() or allocate i 1 bytes.
you must set a null byte at s[i] before you can use strcpy(arr[j], str);
you mist reset i to 0 after storing the word in the array.
str[i] = ch2; after the final loop is incorrect: ch2 has the value EOF, which is not a character. You should set str[i] = '\0' and copy the last line if i != 0.

Here is a modified version:

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

int main() {
    const char *filename = "C:\\Users\\Docs\\lol.txt";
    FILE *fp = fopen(, "r");
    if (fp == NULL) {
        fprintf(stderr, "cannot open file %s: %s\n", filename, strerror(errno));
        exit(8);
    }

    // compute the number of lines and the maximum length
    int len = 0, longest = 0, count = 0;
    for (;;) {
        int c = getc(fp);
        if (c == EOF || c == '\n') {
            if (longest < len)
                longest = len;
            if (len > 0)
                count  ;
            len = 0;
            if (c == EOF)
                break;
        } else {
            len  ;
        }
    }

    printf("the longest line has %d bytes\n", longest);
    printf("%d non empty lines\n", count);

    // allocate the array with an extra entry for a NULL terminator
    char **arr = calloc(sizeof(**arr), count   1);
    if (arr == NULL) {
        fprintf(stderr, "cannot allocate memory\n");
        fclose(fp);
        return 1;
    }

    // read the file contents
    char str[thelongest   1];
    int i = 0, j = 0;
    rewind(fp);
    for (;;) {
        int c = getc(fp);
        if (c == '\n' || c == EOF) {
            // store a new word
            if (j != 0) {
                str[j] = '\0';
                arr[i  ] = strdup(str);
                j = 0;
            }
            if (c == EOF)
                break;
        } else {
            str[j  ] = c;
        }
    }
    arr[i] = NULL;
    fclose(fp);

    // do something with the array
    [...]
    // free the array
    for (i = 0; i < count; i  )
        free(arr[i]);
    free(arr);
    return 0;
}

CodePudding user response：

The return value from getc is an int not a char. If you use a char (like you have) then the test for EOF will never work, so you loop forever.

see https://linux.die.net/man/3/getc

You need

int ch1; <<<<====
int counter1 = 0, i = 0;

while ((ch1 = getc(fp)) != EOF)
{

also counter is not declared anywhere

This line is very wrong

**arr = (char**)malloc(sizeof(char*) * counter);

you mean

arr = (char**)malloc(sizeof(char*) * counter);

this line is missing a ','

int j = 0 i = 0;

but you already have an i declared earlier

in your reading loop you never 0 terminate 'str'

now reading the logic.

YOu cant seem to make up your mind if counter1 is the number of lines in the file or the length of the current line. In the first read loop it is for sure line length.

But then you use it to loop over the array 'length'. Which is sized to the number of lines. Actually not, its sized to 4 - which is wrong too.

YOu would make life easier if you had 'numberOfLines' and 'currentLineLength'

CodePudding user response：

For starters instead of these declarations

char ch1;

and

char ch2;

you should use

int ch1;

and

int ch2;

Otherwise the program will not work correctly if the type char behaves as the type unsigned char.

The variable i is redeclared in this declaration.

int j = 0 i = 0;

It is previously declared in this declaration

int counter1 = 0, i = 0;

So the compiler shall issue an error.

There is another typo

 for (i = 0; i < counter; i  )

The variable counter is not declared.

If the last line in the file also ended with the new line character '\n' then the statement after the

while( (ch1 = getc(fp))!= EOF )
{
    if (ch1 == '\n')
    {
    length[i]=counter1;
    i  ;
    counter1=0;
    continue;
    }
    counter1  ;
}
length[i]=counter1;

can invoke undefined behavior due to accessing memory beyond the array length.

The left operand in this statement

**arr = (char **)malloc(sizeof(char *) * counter);

has the type char. It seems you mean

arr = (char **)malloc(sizeof(char *) * counter);

You need add 1 to the size of this array to reserve space for the terminating zero character '\0'.

char str[thelongest   1];

and the same valid for this memory allocation

arr[j] = (char *)malloc( ( thelongest   1 ) * sizeof(char));

In this if statement

    if (ch2 == '\n')
    {
        arr[j] = (char *)malloc(thelongest * sizeof(char));
        strcpy(arr[j], str);
        // printf("\n%c",str[i]);
        j  ;
        continue;
    }

you forgot to append the terminating zero character

str[i] = '\0';

and to set i to 0.

And again these statements after the loop

str[i] = ch2;
strcpy(arr[j], str);

can invoke undefined behavior if the last word in the file is ended with the new line character '\n'.

And moreover the value of ch2 after the loop is equal to -1 (if the type char behaves as the type signed char) due to the condition of the loop

while ((ch2 = getc(fp)) != EOF)

So this assignment

str[i] = ch2;

does not make a sense.