Why does fscanf ignore the first string?

Question

I am trying to make a translator. This is the part where I put all the strings from the text file on the memory. But the program ignores the first string of the text file.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct b
{
   char b[30];
}b;
int main()
{
    int d,c,i=0;
    char k[30],x;
    b *a;
    FILE *fp;
    if ((fp=fopen("translate.txt","r"))==NULL)
    {
       printf("Σφάλμα κατά το άνοιγμα του αρχείου\n");
    }
    else
    {
        while(!feof(fp))
        {
            fscanf(fp,"%s",k);
            i++;
        }
        a=malloc((i)*(sizeof(b)));
        fclose(fp);
    }
    if ((fp=fopen("translate.txt","r+"))==NULL)
    {
       printf("Σφάλμα κατά το άνοιγμα του αρχείου\n");
    }
    else
    {
        rewind(fp);
        for (c=0;c<i;c++)
       {
        fscanf(fp,"%s",a[c].b);
       }
       fclose(fp);
    }

What do you mean by ignore? You read the string to the buffer `k` and read the next string. Which will over write the first read string — Gopi, Jan 22 '16 at 05:18
@Gopi The first fscanf is used to count the number of words on the text file. — Mitsos, Jan 22 '16 at 05:20
What was confirmed doing "ignores the first string of the text file"? — BLUEPIXY, Jan 22 '16 at 05:22
@BLUEPIXY I am trying to make a translator.Firstly I load the content of the text file to the memory, make changes(add-delete-edit words) and then load them again to the text file.But when I load them, the first word is missing — Mitsos, Jan 22 '16 at 05:25
In your code shown of it does not exist part that you can check it. — BLUEPIXY, Jan 22 '16 at 05:26
@BLUEPIXY The total code is over 500 lines so I didn't put all of it here. — Mitsos, Jan 22 '16 at 05:29
There is no reason why the first word is ignored by this code. It did not reproduce. — BLUEPIXY, Jan 22 '16 at 05:31
@BLUEPIXY That works as ameyCU answered but I don't know why. — Mitsos, Jan 22 '16 at 05:47
Perhaps, what is happening is a buffer overrun exists 30 or more characters of the word in translate.txt. I think so that there is a need to change the size of the buffer to the appropriate size. — BLUEPIXY, Jan 22 '16 at 05:51
See [`while (!feof(file))` is always wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) for a discussion of why you should not use the loop you show. — Jonathan Leffler, Jan 22 '16 at 06:27
when writing code, always use meaningful names. names like d, c, i, k, x, a are not meaningful. In general, a variable name should either express its' usage or its' content. — user3629249, Jan 22 '16 at 21:23
the `typedef` can be understood by a modern compiler, but for us humans, the tag name, the field name, and the typedef name being all the same leads to confusion. Suggest using meaningful, unique names — user3629249, Jan 22 '16 at 21:24
when a call to `fopen()` fails, it is much better to call `perror()` as that will (on stderr) output both the enclosed text and the message from the OS as to why it thinks the call to `fopen()` failed. — user3629249, Jan 22 '16 at 21:27
regarding this line: `fscanf(fp,"%s",k);` 1) always check the returned value (not the parameter value) to assure the operation was successful 2) the '%s' input/conversion specifier was not given a limit, to the input buffer `k[]` can be easily overrun, resulting in undefined behaviour. Suggest: (remembering that scanf() with a '%s' will always append a NUL byte) `if( 1 != fscanf(fp,"%29s",k) ) { handle error } — user3629249, Jan 22 '16 at 21:32
when compiling, always enable all the warnings, then fix those warnings. (for gcc, at a minimum use: `-Wall -Wextra -pedantic` (I also use: `-Wconversion -std=c99` ), The posted code causes the complier to output a long string of error/warning messages. These messages indicate problems in the code that need to be fixed. — user3629249, Jan 22 '16 at 21:35
the call to `malloc()` causes the compiler to raise a warning about the implicit conversion from int to unsigned long. Suggest: `a=malloc( (size_t)i * sizeof(b) );` — user3629249, Jan 22 '16 at 21:39
the system function: `rewind()` is really designed for tape drives, For disk files, a much better function is: `fseek()`. Why call `rewind()`? the file was just opened, so the 'file pointer' will be at the beginning of the file. — user3629249, Jan 22 '16 at 21:40
for readability and ease of understanding by us humans, please consistently indent the code. — user3629249, Jan 22 '16 at 21:43
the overall code can be greatly simplified by 1) calling `realloc()` to expand an initial size for the memory allocation pointed to by `a`. 2) using `fgets()` to read each line from the file, then replacing the newline chars with a NUL byte. The call to `fgets()` can easily be used for loop control. — user3629249, Jan 22 '16 at 21:48
if the first call to `fopen()` fails, but the second call succeeds, then the loop after the second call will: 1) be using an uninitialized memory pointer `a` 2) be using an uninitialized local variable `c` — user3629249, Jan 22 '16 at 21:53
the posted code contains some 'magic' numbers. I.E 30. 'magic' numbers make the code much more difficult to understand, debug, maintain. Suggest: using a #define to give the 'magic' numbers meaningful names, then use those meaningful names throughout the code. — user3629249, Jan 22 '16 at 21:56

score 2 · Accepted Answer · edited Jan 22 '16 at 06:31

1. You should write this loop (so as to check return of fscanf ) —

  for (c=0;c<i;c++)
  {
    fscanf(fp,"%s",a[c].b);
  }

as —

c=0;
while (fscanf(fp,"%29s",a[c].b) == 1 && c<i){
 ...
 c++;
}

2. Also while(!feof(fp)) is wrong, so instead use fscanf to control the loop —

while (fscanf(fp,"%29s",k)==1)
  i++;

Note — And, just to avoid confusion, give different names to your structure member and structure.

score 0 · Answer 2 · answered Jan 22 '16 at 22:18

after applying all the comments, the resulting code is:

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



int main( void )
{
    char **lines = NULL;
    size_t lineCount = 0;
    FILE *fp = NULL;

    if ((fp=fopen("translate.txt","r"))==NULL)
    {
       perror("Σφάλμα κατά το άνοιγμα του αρχείου\n");
       exit( EXIT_FAILURE );
    }


    char * inputBuf = NULL;
    size_t inputLen = 0;

    while( getline( &inputBuf, &inputLen, fp ) )
    {
        lineCount++;
        char **tempLines = realloc( lines, (lineCount+1)*sizeof( char*) );
        if( !tempLines )
        { // then realloc failed
            perror( "realloc failed");
            free( lines );
            exit( EXIT_FAILURE );
        }

        lines = tempLines;
        lines[lineCount] = strdup( inputBuf );
        lineCount++;
    }

    fclose(fp);
    free( lines );
    return 0;
} // end function: main

however, this code is not very efficient as it repeatedly calls realloc()

To fix that, initially allocate enough room in lines[] for several lines, say 10, then keep a count of how many of those pointers are being used.

When all the allocated pointers are used and need to add another line, then double the allocation via realloc().

Why does fscanf ignore the first string?

2 Answers2