-1

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))

will return

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.

DaBler
  • 2,602
  • 2
  • 28
  • 43
CbeginnerXO
  • 33
  • 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

2 Answers2

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
  • 8,727
  • 12
  • 54
  • 100
  • 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
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
  • 265,791
  • 20
  • 170
  • 303