File reading wrong value

Question

I have no clue why this is happening. I have a struct that gets printed to a file perfectly fine, but when I try to read out that file I get something not even close to what is actually there. The write/read portion:

int save_data(){
    char enter = 0;
    int response;
     while(true){
            printf("1. Add new player\n2. View player data\n3. Return to main");
            printf("\nSelection: ");
            scanf("%d", &response);
                if (response == 1){
                     FILE* PlayerFile = fopen("players.txt","w");
                     int i = 0;

                     for (i = 0; i < 1; i++){
                     struct player_info aPlayer = create_player();
                     fprintf(PlayerFile, "Name: %s\nLevel: %d\nStrength Mod: %d\nDexterity Mod: %d\nConstitution Mod: %d\nIntelligence Mod: %d\nWisdom Mod: %d\nCharisma Mod: %d\n", aPlayer.name, aPlayer.Level, aPlayer.Str, aPlayer.Dex, aPlayer.Con, aPlayer.Int, aPlayer.Wis, aPlayer.Cha);
                     }
                     fclose(PlayerFile);
                     return 0;
                     }

                else if (response == 2){
                    struct player_info aPlayer;{
                        char name[30];
                        int Level, Str, Dex, Con, Int, Wis, Cha;
                        };
                    FILE* PlayerFile = fopen("players.txt","r");
                    for (int i = 0; i < 1; i++){
                        struct player_info create_player;
                        fscanf(PlayerFile, "Name: %s\nLevel: %d\nStrength Mod: %d\nDexterity Mod: %d\nConstitution Mod: %d\nIntelligence Mod: %d\nWisdom Mod: %d\nCharisma Mod: %d\n", aPlayer.name, &aPlayer.Level, &aPlayer.Str, &aPlayer.Dex, &aPlayer.Con, &aPlayer.Int, &aPlayer.Wis, &aPlayer.Cha);
                        printf("\nName: %s\nLevel: %d\nStr mod: %d\nDex mod: %d\nCon mod: %d\nInt mod: %d\nWis mod: %d\nCha mod: %d\n\n", aPlayer.name, &aPlayer.Level, &aPlayer.Str, &aPlayer.Dex, &aPlayer.Con, &aPlayer.Int, &aPlayer.Wis, &aPlayer.Cha);
                    }

                    fclose(PlayerFile);
                }
                else if (response == 3){
                false;
                break;}
}
}

The input that is actually in the file:

Name: Hamfast Drummond
Level: 9
Strength Mod: 8
Dexterity Mod: 7
Constitution Mod: 6
Intelligence Mod: 5
Wisdom Mod: 4
Charisma Mod: 3

The result when printing the above:

Name: Hamfast
Level: 6422172
Str mod: 6422176
Dex mod: 6422180
Con mod: 6422184
Int mod: 6422188
Wis mod: 6422192
Cha mod: 6422196

Now, I realize that the one line is missing the & before all the variables, but when I add that in what it prints to the file is the same wrong numbers and reading out is correct in that case, but It's not printing to the file what I put in.

As a side question, if I'm allowed two in one, I'd also like it to print the full name rather than just the first, but I'm not sure how to do that.

When `response == 3` you need `return false;`, not just `false;`. — Paul R, Feb 20 '17 at 07:17
That would remove the need for the break afterwards. I'd been wondering why it hadn't been working without the break. Thank's for clearing that up. — hego64, Feb 20 '17 at 07:20
unused `struct player_info create_player;` and `{ char name[30]; int Level, Str, Dex, Con, Int, Wis, Cha; };` — bansi, Feb 20 '17 at 07:23
@bansi Do those not need to be in there then? I'd included them because I wasn't sure if they were necessary or not, just to be safe. — hego64, Feb 20 '17 at 07:27
just comment those and see for yourselves. also `for (int i = 0; i < 1; i++)` loop is never required (thinking you are keeping it there for later when `i<1` is not the condition) — bansi, Feb 20 '17 at 07:31
@bansi I'll do that. And yes, It's there for when I figure I'll add more to it later. — hego64, Feb 20 '17 at 07:32
Just for info: alternatively you can read the entire structure in one go from the file using fread. check [How to fread() structs?](http://stackoverflow.com/questions/21721808/how-to-fread-structs) — bansi, Feb 20 '17 at 07:38
@bansi I'll definitely check that out. That could be useful if I can figure it out, haha. Thanks for that. Also, you were right, with slight modification those bits were unnecessary and I've removed them. Again, thanks for everything. — hego64, Feb 20 '17 at 07:44
when calling any of the `scanf()` family of functions, always check the returned value (not the parameter values) to assure the operation was successful. — user3629249, Feb 20 '17 at 19:25
when calling `fopen()`, always check (!=NULL) the returned value to assure the operation was successful. — user3629249, Feb 20 '17 at 19:26
for ease of readability and understanding: 1) consistently indent the code. indent after every opening brace '{'. unindent before every closing brace '}'. Suggest each indent level be 4 spaces as that is wide enough to be visible even with variable width fonts and allows for many levels across the page. — user3629249, Feb 20 '17 at 19:28
at this point in the posted code: `struct player_info aPlayer = create_player();`, the `struct player_info` type is not defined. — user3629249, Feb 20 '17 at 19:31
this statement: `for (i = 0; i < 1; i++)` is completely unneeded and can be removed, including the associated braces. Note: the call to `create_player()` should be returning a pointer to a malloc'd player instance. but the declaration `struct player_info` is not defined as a pointer. And there is no passing of that pointer to `free()` — user3629249, Feb 20 '17 at 19:35
to help make the posted code compile, move the definition of `struct player_info` to before the function: `save_data()` — user3629249, Feb 20 '17 at 19:37
the values `true` and `false` are not defined in the posted code. Perhaps the code should have the statement: `#include ` — user3629249, Feb 20 '17 at 19:42
the variable `enter` is declared and initialized, but never used. — user3629249, Feb 20 '17 at 19:43
this statement: `printf( "\nName: ..... )` is passing the address of the fields to the function, rather than the contents of the field. This is one of the main reasons the display of the file contents is failing. — user3629249, Feb 20 '17 at 19:49
the function: `save_data()` signature says it returns an `int` but the posted code does not return anything. — user3629249, Feb 20 '17 at 19:51
within the `while(true)` loop, the file is being opened for `write` via the call to `fopen()` the result is the file is truncated each time that call is made. Perhaps you meant to use "a"` for that mode rather than "w". — user3629249, Feb 20 '17 at 19:53

score 4 · Answer 1 · answered Feb 20 '17 at 07:20

printf wants values with %d format specifiers, while scanf wants references

This:

printf("\nName: %s\nLevel: %d\nStr mod: %d\nDex mod: %d\nCon mod: %d\nInt mod: %d\nWis mod: %d\nCha mod: %d\n\n", aPlayer.name, &aPlayer.Level, &aPlayer.Str, &aPlayer.Dex, &aPlayer.Con, &aPlayer.Int, &aPlayer.Wis, &aPlayer.Cha);

Must be

printf("\nName: %s\nLevel: %d\nStr mod: %d\nDex mod: %d\nCon mod: %d\nInt mod: %d\nWis mod: %d\nCha mod: %d\n\n", aPlayer.name, aPlayer.Level, aPlayer.Str, aPlayer.Dex, aPlayer.Con, aPlayer.Int, aPlayer.Wis, aPlayer.Cha);

In other words you are (badly) printing addresses of those vars, not their values.

score 3 · Answer 2 · answered Feb 20 '17 at 07:20

printf("\nName: %s\nLevel: %d\nStr mod: %d\nDex mod: %d\nCon mod: %d\nInt mod: %d\nWis mod: %d\nCha mod: %d\n\n", aPlayer.name, aPlayer.Level, aPlayer.Str, aPlayer.Dex, aPlayer.Con, aPlayer.Int, aPlayer.Wis, aPlayer.Cha);

You are printing address of what you have read from file

Karthik Nayak · Accepted Answer · 2017-02-20T07:35:17.517

2

About reading the whole name:

use the rule %[^\n]\n instead of %s\n. This would read everything apart from a newline, giving you the entire name.

printf shouldn't use the &. The & is used to obtain the address of a given variable. So only while using scanf use the &, because we want to store the value in the address. While using printf use only the variable name, cause you want the value.

edited Feb 20 '17 at 07:35

answered Feb 20 '17 at 07:22

Karthik Nayak

731
3
14

Thank you for clearing the name issue up, and when to use `&`. As per usual after asking something, I feel a bit dumb for not noticing it myself, but now I know and can do it right next time 'round. So many good answers here. Thanks all for helping and for the quick replies! Got it fixed now. – hego64 Feb 20 '17 at 07:26
Happens to all of us, what you could do is try and read the errors the compilers throw and see if you can debug using that. – Karthik Nayak Feb 20 '17 at 07:33
I wasn't getting any compiler errors in this though, as it was technically correct, and just wasn't doing what I had thought it should. Though I should probably use some error extenders or whatever like `-pedantic` and some others I've seen. Not sure what they all do exactly, but it can't hurt to get the extra info. – hego64 Feb 20 '17 at 07:36
Yeah, they're a pain.Thanks for everything again. – hego64 Feb 20 '17 at 07:42

user3629249 · Answer 4 · 2017-02-20T20:13:26.877

the posted code contains many problems, as outlined in the other answers and in the comments to the question.

Here is a version of the code that cleanly compiles and implements the suggest corrections:

#include <stdio.h>
#include <stdbool.h>



struct player_info
{
    char name[30];
    int Level, Str, Dex, Con, Int, Wis, Cha;
};

struct player_info create_player( void );

int save_data( void );

int save_data( void )
{
    //char enter = 0;
    int response;

     while(true)
     {
            printf("1. Add new player\n"
                   "2. View player data\n"
                   "3. Return to main");
            printf("\nSelection: ");

            if( 1 != scanf("%d", &response) )
            {
                perror( "scanf failed" );
                return 1;
            }

            if (response == 1)
            {
                 FILE* PlayerFile = fopen("players.txt","r");
                 if( NULL == PlayerFile )
                 {
                     perror( "fopen for truncate and write failed" );
                     return 1;
                 }

                 int i = 0;

                 struct player_info aPlayer = create_player();
                 fprintf(PlayerFile, "Name: %s\nLevel: %d\nStrength Mod: %d\nDexterity Mod: %d\nConstitution Mod: %d\nIntelligence Mod: %d\nWisdom Mod: %d\nCharisma Mod: %d\n", aPlayer.name, aPlayer.Level, aPlayer.Str, aPlayer.Dex, aPlayer.Con, aPlayer.Int, aPlayer.Wis, aPlayer.Cha);

                 fclose(PlayerFile);
                 return 0;
            }

            else if (response == 2)
            {
                struct player_info aPlayer;

                FILE* PlayerFile = fopen("players.txt","r");
                if( NULL == PlayerFile )
                {
                    perror( "fopen for reading failed" );
                    return 1;
                }

                fscanf(PlayerFile, "Name: %s\nLevel: %d\nStrength Mod: %d\nDexterity Mod: %d\nConstitution Mod: %d\nIntelligence Mod: %d\nWisdom Mod: %d\nCharisma Mod: %d\n",
                    aPlayer.name,
                    &aPlayer.Level,
                    &aPlayer.Str,
                    &aPlayer.Dex,
                    &aPlayer.Con,
                    &aPlayer.Int,
                    &aPlayer.Wis,
                    &aPlayer.Cha);
                printf("\nName: %s\nLevel: %d\nStr mod: %d\nDex mod: %d\nCon mod: %d\nInt mod: %d\nWis mod: %d\nCha mod: %d\n\n",
                    aPlayer.name,
                    aPlayer.Level,
                    aPlayer.Str,
                    aPlayer.Dex,
                    aPlayer.Con,
                    aPlayer.Int,
                    aPlayer.Wis,
                    aPlayer.Cha);


                fclose(PlayerFile);
            }

            else if (response == 3)
            {
                //false;
                break;
            }
    }
    return 0;
}

however, I would strongly suggest NOT placing all the field label text in the file, as it just creates problems with trying to read the file from within the program.

Notice that the program returns after any single operation, so the file cannot contain more that the info on a single player.

File reading wrong value

4 Answers4