Thread: Passing Structures as Function Arguments

1. Move your typedef's outside main. As written, you've limited the scope of the typedef's to main().

2. Make your function prototypes the same as the function definition.

Lines 4 to 14 are types.
You don't create any data storage at this point.

Lines 16 and 17 are instances of those types.
These allocate some actual space. It's generally a good idea to restrict the scope of data to the narrowest scope which makes sense.

You should move the typedefs and function prototypes outside of main. These are not global "objects" but declarations of types.

For function TXTDATA_Read_Users:
The first formal parameter should be a pointer (you're missing the asterisk). Also, if you want to access members User, Pass, Role, then presumably it should be DATATYPE_Users not DATATYPE_Rates.

And you're missing two variables: COUNT_Struct_Rates and COUNT_Struct_Users.

Also, when you call your two functions, don't use the ampersand in front of the first parameters. They are already pointers.

These

Code:

    DATATYPE_Rates STRUCT_Rates[100];
    DATATYPE_Users STRUCT_Users[100];

can stay in main. They will be allocated on main's stack and passed (as pointers) to their respective functions.

Other points:
There's (apparently) no reason for you to be passing in the FILE*'s. Declare them locally in the functions.
You should close your files at the end of those functions.
main should be declared as int main(void) and should have a return 0; as it's last line.

Originally Posted by Jyqft

@oogabooga: Regarding the files. I plan on closing them in another function (which I've not yet written). So that's why I want to have them in int main();, so I can pass the FILE*s and close them.

Well yeah but it's not a bad idea to close them in the meantime as well. That's like saying, I will leave this stove on and will come back to it and turn it off after I have cooked my main meal on this other stove.

Originally Posted by Jyqft

Regarding the files. I plan on closing them in another function

Why?

The more one looks, the more errors one finds:

• If the file does not exist, don't print an error message AND THEN continue with normal processing.
• You're not checking for end-of-file properly.
• You are both setting a passed in value (through a pointer) AND returning the value. Just return it, since it looks like there's no reason to pass it in.
• You're not setting COUNT_Struct_Rates and COUNT_Struct_Users.
• And if you tell us what your data looks like there might be a better way to read it.

To sum up:
You've written more than you should have without stopping to test it.

@claudiu: Fair enough.

@oogabooga:
1. Regarding handling of the file's existence: I'm not sure how to interrupt normal processing. What do you think should I do?
2. Regarding EOF checking: How so?
3. Regarding passing AND returning: Point. I was actually wondering how come no one pointed that out (until now). But if I choose to return only, should I simply equate the COUNT variables to the function in int main();?
4. Regarding Setting the COUNT variables: But if you check the TXTDATA functions, I set them to i. Would it matter if I didn't set them to 0 at first?
5. Umm I'm not sure what you mean. But this is, after all, a fragment of my source code. The objective of the program I'm writing is to deal with currencies, if that helps.

By the way, I have stopped to test my code. It just so happens that my organization is pretty lousy...

1.

Code:

FILE *fp = fopen("file.dat", "r");
if (!fp) {
    fprintf(stderr, "Can't open file.dat\n");
    exit(1);  // exit the program (returning 1 to the OS)
}
// ...
fclose(fp);

2.

Code:

int rv; // fscanf returns the number of values it reads
while (1) {
    rv = fscanf(fp, "%d", &n[i].x);
    if (rv != 1) break;
    rv = fscanf(fp, "%d", &n[i].y);
    if (rv != 1) break;
    i++;
}

3. Yes.
4. Just return i from the functions and define and equate the COUNT vars to them in main.
5. You don't understand what it means to describe the format of your data? Never mind then.

So your function prototypes should just be

Code:

int TXTDATA_Read_Rates(DATATYPE_Rates *a);
int TXTDATA_Read_Users(DATATYPE_Users *STRUCT_Users);

(although a is not a very good name).

I'm down with this:

Code:

int TXTDATA_Read_Rates(DATATYPE_Rates *STRUCT_Rates) {
    
    char FloatVal[99];
    int i;
    
    FILE *STREAM_Rates;
    
    STREAM_Rates = fopen("rates.txt", "rt");
    
    if (STREAM_Rates == NULL) {
        printf("[Error!] \"rates.txt\" does not exist.\n");
        exit(1);
    }
    
    for (i = 0; !feof(STREAM_Rates); i++) {
        
        fscanf(STREAM_Rates, "%[^\t\n]\n", STRUCT_Rates[i].OrigCurr);        
        fscanf(STREAM_Rates, "%[^\t\n]\n", STRUCT_Rates[i].WantCurr);        
        fscanf(STREAM_Rates, "%[^\t\n]\n", FloatVal);
        STRUCT_Rates[i].ExRate = atof(FloatVal);
        
    }
    
    fclose(STREAM_Rates);
    return i;
    
}


int TXTDATA_Read_Users(DATATYPE_Users *STRUCT_Users) {
    
    int i;
    
    FILE *STREAM_Users;
            
    STREAM_Users = fopen("CA_users.txt", "rt");
        
    if (STREAM_Users == NULL) {
        printf("[Error!] \"CA_users.txt\" does not exist.\n");
        exit(1);
        
    }    
    
    for (i = 0; !feof(STREAM_Users); i++) {
        
        fscanf(STREAM_Users, "%[^\t\n]\n", STRUCT_Users[i].User);
        fscanf(STREAM_Users, "%[^\t\n]\n", STRUCT_Users[i].Pass);
        fscanf(STREAM_Users, "%[^\t\n]\n", STRUCT_Users[i].Role);
        
    }
    
    fclose(STREAM_Users);
    return i;
    
}

But it gives me Segmentation fault 11. What am I doing wrong now?

EDIT: Never mind, another part of the code was causing the problem.

This is probably what oogabooga was referencing:

Code:

int TXTDATA_Read_Rates(DATATYPE_Rates *STRUCT_Rates) {
     
    char FloatVal[99];
    int i;
     
    FILE *STREAM_Rates;

    ...

    for (i = 0; !feof(STREAM_Rates); i++) {
         
        fscanf(STREAM_Rates, "%[^\t\n]\n", STRUCT_Rates[i].OrigCurr);       
        fscanf(STREAM_Rates, "%[^\t\n]\n", STRUCT_Rates[i].WantCurr);       
        fscanf(STREAM_Rates, "%[^\t\n]\n", FloatVal);
        STRUCT_Rates[i].ExRate = atof(FloatVal);
         
    }
     
    ...
     
}
 
 
int TXTDATA_Read_Users(DATATYPE_Users *STRUCT_Users) {
     
    int i;
     
    FILE *STREAM_Users;

    ...
     
    for (i = 0; !feof(STREAM_Users); i++) {
         
        fscanf(STREAM_Users, "%[^\t\n]\n", STRUCT_Users[i].User);
        fscanf(STREAM_Users, "%[^\t\n]\n", STRUCT_Users[i].Pass);
        fscanf(STREAM_Users, "%[^\t\n]\n", STRUCT_Users[i].Role);
         
    }
  
    ...

}

You should not use an EOF test to control your loops.

Read: FAQ > Why it's bad to use feof() to control a loop

The link uses a while(!feof(stream)) loop to demonstrate what's wrong however your for loop is basically doing the same thing.

There are different ways of handling this, you've been shown one way. Another way would be to consider that you are reading three values in the body of the loop with three individual fscanf calls. Your for loop should be making a single fscanf call reading in three values and testing that 3 values were scanned. Your loop should also make sure that i is less than 100 before you attempt to read data into an array index that might not be valid.