passing structures among functions

[sofeng]

The following link shows a chart I created about passing structures
among functions. Would you review it and tell me if it requires any
corrections?

http://bp2.blogger.com/_lZhqNsiakm4/Reh26hy-JHI/AAAAAAAAAAk/wvyV3Yx8gSs/s1600-h/gif_1.gif

Thank you.
sofeng

 

[Arthur J. O'Dwyer]

The following link shows a chart I created about passing structures
among functions. Would you review it and tell me if it requires any
corrections?

http://bp2.blogger.com/_lZhqNsiakm4/Reh26hy-JHI/AAAAAAAAAAk/wvyV3Yx8gSs/s1600-h/gif_1.gif

FYI, many readers of Usenet *will not* go to a Web page just because
a pseudonymous poster asks them to, especially when the page has such
an obviously machine-generated URL. (And perhaps doubly so when the
URL is actually the URL of an HTML page, despite the ".gif" ending.
That's just unnecessary.)

Note to prospective Web-goers: I used 'wget' to grab the GIF, so if
there's any malicious code on that Web page, I didn't encounter it.
Don't think I'm endorsing the page.

Okay, now on to your question. Mistakes ordered from most significant
to least significant:

* All your ".h" files are missing the include guards.
http://en.wikipedia.org/wiki/Include_guard
* "get_data.h" refers to DATA, so it needs to #include "defs.h".
* "get_data.c" refers to get_data(), so it needs to #include "get_data.h".
* "DATA", in all caps, looks like a preprocessor macro. It is better to
use something non-macro-ish, such as "Data" or "data_t". Except...
* The use of "typedef" is superfluous, and most comp.lang.c regulars
will counsel you not to use it. Give the struct a tag, such as "struct
data", and use that instead.
* You spelled "definitions" as "definitons" in one place.
* "float" is rarely used; "double" would be more newbie-friendly, if your
goal is to show what real code looks like.
* "Header files should only contain declarations." should read
"Header files should contain only declarations." (And macro definitions,
and comments.)
* Burn your GIFs and use PNG instead; it's cooler.

HTH,
-Arthur

 

[Ian Collins]

Note to prospective Web-goers: I used 'wget' to grab the GIF, so if
there's any malicious code on that Web page, I didn't encounter it.
Don't think I'm endorsing the page.

Better to just avoid the windows/IE combination

 

[sofeng]

FYI, many readers of Usenet *will not* go to a Web page just because
a pseudonymous poster asks them to, especially when the page has such
an obviously machine-generated URL. (And perhaps doubly so when the
URL is actually the URL of an HTML page, despite the ".gif" ending.
That's just unnecessary.)

Note to prospective Web-goers: I used 'wget' to grab the GIF, so if
there's any malicious code on that Web page, I didn't encounter it.
Don't think I'm endorsing the page.

Okay, now on to your question. Mistakes ordered from most significant
to least significant:

* All your ".h" files are missing the include guards.http://en.wikipedia.org/wiki/Include_guard
* "get_data.h" refers to DATA, so it needs to #include "defs.h".
* "get_data.c" refers to get_data(), so it needs to #include "get_data.h".
* "DATA", in all caps, looks like a preprocessor macro. It is better to
use something non-macro-ish, such as "Data" or "data_t". Except...
* The use of "typedef" is superfluous, and most comp.lang.c regulars
will counsel you not to use it. Give the struct a tag, such as "struct
data", and use that instead.
* You spelled "definitions" as "definitons" in one place.
* "float" is rarely used; "double" would be more newbie-friendly, if your
goal is to show what real code looks like.
* "Header files should only contain declarations." should read
"Header files should contain only declarations." (And macro definitions,
and comments.)
* Burn your GIFs and use PNG instead; it's cooler.

HTH,
-Arthur

Thank you much for the feedback. I have made the changes suggested.
How does it look now? Here is the link for the png file:
http://bp1.blogger.com/_lZhqNsiakm4/RezJubUauVI/AAAAAAAAAA4/nYIxnx9E1Z8/s1600-h/png_1.png
The text contained in the diagram is below:

main.c:
- main.c contains the main function which calls the other functions
that operate on the data structure.
- It also defines (allocates memory) for the data structure at the
file level.
- The data structure is declared using the static keyword so that it
will have static duration (i.e. it will exist from program start to
finish), but it will have internal linkage (i.e. it won't be global).
- The address operator, &, is used to create a pointer to the data
structure which is passed to the other functions.

#include "defs.h"
#include "get_data.h"
#include "modify_data.h"

static struct data the_data;

int main()
{
get_data(&the_data);
modify_data(&the_data);

return 0;
}

get_data.c:
- get_data() fills the data structure
- it is passed a pointer to the data structure defined in main.c

#include "defs.h"
#include "get_data.h"

int get_data(struct data *data_ptr)
{
data_ptr->item1 = 1;
data_ptr->item2 = 2;
data_ptr->item3 = 3.0;
data_ptr->item4 = 4.0;

return 0;
}

modify_data.c:
- modify_data() modifies the data structure
- It is similar to get_data()

#include "defs.h"
#include "modify_data.h"

int modify_data(struct data *data_ptr)
{
data_ptr->item2 += 1;
data_ptr->item4 += 1.0;

return 0;
}

defs.h:
- contains the structure data type declaration

#ifndef DEFS_H_
#define DEFS_H_

struct data
{
int item1;
int item2;
double item3;
double item4;
};

#endif /*DEFS_H_*/

get_data.h:
- contains the get_data() function declaration

#ifndef GET_DATA_H_
#define GET_DATA_H_

#include "defs.h"

int get_data(struct data *data_ptr);

#endif /*GET_DATA_H_*/

modify_data.h:
- contains the modify_data() function declaration

#ifndef MODIFY_DATA_H_
#define MODIFY_DATA_H_

#include "defs.h"

int modify_data(struct data *data_ptr);

#endif /*MODIFY_DATA_H_*/

NOTE: Header files should contain only declarations, macro
definitions, and comments. They should not contain variable
definitions because if the header file is included in multiple
locations, there would be multiple definitions of the same variable.

 

[Flash Gordon]

sofeng wrote, On 06/03/07 02:07:

Thank you much for the feedback. I have made the changes suggested.
How does it look now? Here is the link for the png file:
http://bp1.blogger.com/_lZhqNsiakm4/RezJubUauVI/AAAAAAAAAA4/nYIxnx9E1Z8/s1600-h/png_1.png
The text contained in the diagram is below:

I'm still not following the link (too lazy) but since you've included
the code it does not matter

main.c:
- main.c contains the main function which calls the other functions
that operate on the data structure.
- It also defines (allocates memory) for the data structure at the
file level.
- The data structure is declared using the static keyword so that it
will have static duration (i.e. it will exist from program start to
finish), but it will have internal linkage (i.e. it won't be global).
- The address operator, &, is used to create a pointer to the data
structure which is passed to the other functions.

#include "defs.h"
#include "get_data.h"
#include "modify_data.h"

static struct data the_data;

There is no need for this to be a file scope variable. Better would be
to declare it inside main and not static. This will also "hide" it from
any other functions you (or someone else) later write in the source file
containing it.

int main()

As you are not using command line parameters better to be explicit
int main(void)

{

struct data the_data;
Or, if you want it to be initialised
struct data the_data = {0};

get_data(&the_data);
modify_data(&the_data);

return 0;
}

get_data.c:
- get_data() fills the data structure
- it is passed a pointer to the data structure defined in main.c

#include "defs.h"
#include "get_data.h"

int get_data(struct data *data_ptr)

Since this only ever returns the one value why have it return anything
at all? Better would be
void get_data(struct data *data_ptr)
Then loose the return at the bottom.

{
data_ptr->item1 = 1;
data_ptr->item2 = 2;
data_ptr->item3 = 3.0;
data_ptr->item4 = 4.0;

return 0;
}

modify_data.c:
- modify_data() modifies the data structure
- It is similar to get_data()

#include "defs.h"
#include "modify_data.h"

int modify_data(struct data *data_ptr)

Same comment as last one. Better
void modify_data(struct data *data_ptr)

NOTE: Header files should contain only declarations, macro
definitions, and comments. They should not contain variable
definitions because if the header file is included in multiple
locations, there would be multiple definitions of the same variable.

typedefs are also OK in headers when it makes sense to use them, which
it does sometimes.