0

I am trying to read a line from file.txt and return it as string til maxchar without using getline().

For example, if file.txt contains

c language 
language c

Then:

printf("%s\n", get_line(f, 3)) 
printf("%s\n", get_line(f, 3))

should output

c l
lan

But my code returns weird characters.

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

char *get_line(FILE *f, int maxchar) {

    char *string = (char *)malloc(sizeof(f));
    int c;
    int i;

    while ((c = fgetc(f)) != EOF && c != '\n') {
        fgets(string, maxchar, f);
    }
    return string;          
}

int main(void) {
    FILE *f;
    f = fopen("file.txt", "r");
    printf("%s\n %s\n", get_line(f,3), get_line(f,5));

    fclose(f);
    return 0;
}

Is there any wrong in reading file? or using fgetc()?

Any help would be appreciated.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
CbeginnerXO
  • 43
  • 1
  • 5
  • 0) `sizeof(f)` isn't size of file. 1)`c = fgetc(f)` drop one read of character. 3) you don't drop upto newline. – BLUEPIXY Aug 08 '15 at 08:33
  • 1
    Why do you have an `fgets` inside the while loop? You don't even need a `get_line` function if you're allowed to use `fgets`. – user3386109 Aug 08 '15 at 08:35
  • Why so complicated? `fgets` stops already at newline, end-of-file or given limit. – Youka Aug 08 '15 at 08:41

3 Answers3

1

OK, had to test it. More improvements.

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

char* get_line(FILE* f, int maxchar){

    char* string = malloc(maxchar+1);
    int c;
    int i = 0;

    while ((c = fgetc(f)) != EOF && c != '\n' && i < maxchar){
        *(string + i++) = c;
    }
    *(string + i) = '\0';
    return string;
}


int main(void){
    FILE* f;
    f = fopen("file.txt", "r");
    printf("%s\n", get_line(f, 3)); 
    printf("%s\n", get_line(f, 5));

    fclose(f);
    return 0;
}

The reason that:

printf("%s\n %s\n",get_line(f,3),get_line(f,5));

gives you unexpected results is due to order of evaluation of the function arguments. In C which uses cdecl, the actual arguments to printf are evaluated right to left. How printf handles this you would have to look at the code. More reliable would be to use two lines as my example.

Your malloc line was wrong because you need to allocate space for number of characters and sizeof FILE* will return the size of a pointer which is not what you want. You could also have checked the file byte size and use that as your maxchar.

Your use of fgets was also wrong.

In the loop you will have a char array with eg:

'c', ' ', 'l', 'a', 'n'

followed by random values (because you did not pre-assign values to the malloc'd buffer. So the line:

*(string + i) = '\0';

appends a null character to the end of the string to make it a null terminated c string - which is expected by the C string functions.

For clarity, *(string+i) means you take the address of string, add i and dereference (asterisk - * is the dereference operator).

= '\0'

appends the null character on the end of the string.

Angus Comber
  • 9,316
  • 14
  • 59
  • 107
  • 2
    you do not need the cast – Ed Heal Aug 08 '15 at 08:27
  • 1
    `sizeof(maxchar)` is the same as `sizeof(int)` and therefore most likely 4 and has also nothing to do with the byte count of the file. – mch Aug 08 '15 at 08:30
  • [Here's why you don't need to cast the return from malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – user3386109 Aug 08 '15 at 08:38
  • Just add a NULL to terminate what you have read and it will look like the original `fgets` (except that `fgets` keeps the `\n`) – Serge Ballesta Aug 08 '15 at 08:40
  • The function posted does not implement the expected behavior: it should consume the rest of the line if the line is longer than `maxchar`. – chqrlie Aug 01 '23 at 18:19
1

This statement

char* string = (char*)malloc(sizeof(f));

does not make sense. You need to allocate memory for a string of size maxchar.

The same can be said about this loop

while((c = fgetc(f)) != EOF && c != '\n'){
        fgets(string,maxchar,f);

Either you should use fgetc or fgets

The function can be written the following way using fgetc

char * get_line( FILE *f, size_t maxchar )
{
    char *record = NULL;

    if ( maxchar && !feof( f ) && ( record = malloc( maxchar ) ) != NULL )
    {
        int c;
        size_t i = 0;

        while ( i < maxchar - 1 && ( c = fgetc( f ) ) != EOF && c != '\n' ) 
        {
            record[i++] = c;
        }

        record[i] = '\0';
    }

    return record;          
}

And do not forget to free the record in main before the end of the program. For example

char *s;

if ( ( s = get_line(f,3) ) != NULL ) printf( "%s\n", s );
free( s );


if ( ( s = get_line(f,5) ) != NULL ) printf( "%s\n", s );
free( s );

The better approach is to declare the function the following way

char * get_line( FILE *f, char *s, size_t maxchar );

In this case you can declare a character array in main and pass it in the function.

For example

char * get_line( FILE *f, char *s, size_t maxchar )
{
    size_t i = 0;

    if ( maxchar )
    {
        int c;

        while ( i < maxchar - 1 && ( c = fgetc( f ) ) != EOF && c != '\n' ) 
        {
            s[i++] = c;
        }
    }

    s[i] = '\0';

    return s;          
}

And in main there can be

#define MAX_CHAR 80
//...
int main( void )
{
    char s[MAX_CHAR];

    //...

    printf( "%s\n", get_line( f, s, 3 ) );

    printf( "%s\n", get_line( f, s, 5 ) );

    //...
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

There are multiple problems in your code:

  • char *string = (char *)malloc(sizeof(f)); allocates space for a FILE pointer: it is meaningless. You should instead allocate space for the maximum line length, including the null terminator. Note however that this may fail if you pass a huge length, regardless of the maximum line length in the file.

  • you are mixing fgetc() and fgets() in the loop, so some characters will get lost and the only only exits on blank lines or at end of file.

  • the call printf("%s\n %s\n", get_line(f,3), get_line(f,5)); has unspecified behavior because the order of evaluation of function arguments is undefined.

The function get_line can be fixed easily:

  • allocate the result array, or better take the destination array as an argument.
  • read all bytes from the line but only store the initial ones that fit into the destination array.
  • if no bytes were read successfully, signal end of file to the caller by returning NULL.

Here is a modified version:

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

char *get_line(FILE *f, size_t maxchar) {
    char *string = malloc(maxchar + 1);
    if (string) {
        int c;
        size_t i = 0, j = 0;

        while ((c = fgetc(f)) != EOF && c != '\n') {
            if (j < maxchar)
               string[j++] = c;
            i++;
        }
        if (c == EOF && i == 0) {
            /* end of file */
            free(string);
            string = NULL;
        } else {
            string[j] = '\0';
        }
    }
    return string;
}

int main(void) {
    FILE *f = fopen("file.txt", "r");
    if (f != NULL) {
        char *p1;
        printf("%s\n", p1 = get_line(f, 3)); 
        printf("%s\n", p2 = get_line(f, 5));
        free(p1);
        free(p2);
        fclose(f);
    }
    return 0;
}

Here is an alternative approach where you pass the detination array along with its size, as you would for fgets():

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

char *get_line(FILE *f, char *dest, size_t size) {
    int c;
    size_t i = 0, j = 0;

    while ((c = fgetc(f)) != EOF && c != '\n') {
        if (j + 1 < size)
           dest[j++] = c;
        i++;
    }
    if (c == EOF && i == 0) {
        /* end of file */
        return NULL;
    }
    if (j < size) {
        dest[i] = '\0';
    }
    return dest;
}

int main(void) {
    FILE *f = fopen("file.txt", "r");
    if (f != NULL) {
        char buf1[3 + 1];
        char buf2[5 + 1];
        printf("%s\n", get_line(f, buf1, sizeof buf1)); 
        printf("%s\n", get_line(f, buf2, sizeof buf2)); 
        fclose(f);
    }
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189