Re: first program with pointers to structures and realloc....

In article <f2vdi3-0op.ln1@xxxxxxxxxxx>
Martin Joergensen <unoder.spam@xxxxxxxxxxxx> wrote:
... my program won't compile and I don't know what to do about the
warnings/error messages.

c:\documents and settings\dell\Desktop\test\main.c(5) : warning
C4115: 'tablevalues' : named type definition in parentheses

GCC's complaint is perhaps slightly more useful:

warning: `struct tablevalues' declared inside parameter list
warning: its scope is only this definition or declaration,
which is probably not what you want.

c:\documents and settings\dell\Desktop\test\main.c(41) : warning
C4133: 'function' : incompatible types - from 'tablevalues *' to
'tablevalues *'

This particular complaint is just plain wrong, however. One of
the types is indeed "tablevalues *", but the other is "struct
tablevalues *". The difference is that the second one has the
keyword "struct" in front of it, announcing to the world (and the
human programmer) that this is in fact a type-name. The first
looks like an ordinary variable name; but if it were, it would be
a syntax error, so it must be an identifier defined via a type-alias.

[snippage - code fragment follows]

#include <stdlib.h>
#include <stdio.h>

void save_in_memory( struct tablevalues *data_pointer,
unsigned *number_of_sets_in_memory,
unsigned increase_memory_number,
double double_x, int var1,
int var2, int var3);

Here is the first problem: "struct tablevalues" appears here in
parentheses. This is a "named type" (as per to your compiler).
But, as GCC says, the scope of this declaration -- that there
exists a type called "struct tablevalues" -- is limited to the
prototype declaration. Once the prototype ends, the type-name
vanishes, and can never be mentioned again.

This is where things get a little tricky. If you write:

void f(void) {
int x;
...
}
void g(void) {
int x;
...
}

we all understand that the x in f() is quite different from the
x in g(). Both are "int x" but they are not the same variable;
if g() calls into f(), both "x"s become active simultaneously.

This occurs because the two "x"s have different scope.

The same thing occurs with type-names. If you create a new
type:

struct zorg;

this type is name-able until its scope terminates. Then it has
gone out of scope and it can never be named again. Thus:

void f(void) {
struct zorg { int i; };
struct zorg a, b, x, y;
...
}
void g(void) {
struct zorg { double d; char *p; };
struct zorg a, b, x, y;
...
}

Not only are "a", "b", "x", and "y" different in f() and g(), so
are the two different "struct zorg"s! They are both named "struct
zorg", but they have different contents and are in fact different
types.

As with variables, it is usually a bad idea to make a type-name do
double duty in inner vs outer scopes. Consider the following code
fragment:

int bang;
void f(void) {
double bang;
...
}

Suppose I now say something beginning with: "when bang is negative".
Which "bang" am I talking about? Obviously I will have to be
more specific.

The same holds for type-names (which we define with the "struct"
keyword, as usual):

struct zorg { int i; };
void g(void) {
struct zorg { double d; char *p; };
...
}

In this case, if I say something about "when the d member of a zorg
is negative", it is clear enough that I mean the zorg in g(), and
not the outer zorg, because only inner-zorg has a "d". But it is
still not a great idea to allow for this kind of confusion.

The problem you have, at this point, is that you are trying to
declare the function save_in_memory(), complete with a prototype,
but the type "tablevalues" has not yet been declared. The solution
is simple: declare the type. Then the inner-scope name in the
prototype will refer to the *same* type that is already declared
in the outer scope. This is just like *not* declaring a new
"struct zorg" inside g(). So now we have:

struct tablevalues; /* This just declares the type. */

void save_in_memory(struct tablevalues *data_pointer,
unsigned int *number_of_sets_in_memory,
unsigned int increase_memory_number,
double double_x, int var1, int var2, int var3);

We proceed onward:

/* ************ Main program ************ */

[this comment is a little redundant :-) ]

int main()
{

[better to write "int main(void) { ... }", so as to provide a
prototype along with the definition, but this is OK]

const unsigned increase_memory_number = 3;
unsigned number_of_sets_in_memory;

[I prefer to write out "unsigned int", but this is also OK]

double doub_x;
int va1, va2, va3;

typedef struct
{
double double_x;
int var1;
int var2;
int var3;
} tablevalues;

Here we have a problem: this creates a new, unnamed type -- we
wrote "struct {" without using a tag -- and then "typedef"s an
alias for that unnamed type. (Remember, typedef *never* defines
a new type; it only makes aliases for existing types.) So this
produces a type-alias named "tablevalues" that is an alias for
a structure with no tag. But in fact, it is obvious to at least
one particular human -- though not to a computer -- that you
wanted to use the *same* structure type here as in your function
save_in_memory().

You could attempt to repair this by doing something like:

typedef struct tablevalues tablevalues;
struct tablevalues { ... contents ... };

(assuming you insist on using the useless "typedef", otherwise
you could omit that line entirely). But this is doomed to fail:
the syntax for "define a new type for me" is to write:

struct tagname_opt { ... contents ... };

so this means "define a *new*, inner-scope type named struct
tablevalues". The typedef-name "tablevalues" will now refer
to the old, outer-scope "struct tablevalues", while the new,
inner-scope "struct tablevalues" is a new and different type,
regardless of the "contents" part.

The only solution is to define (not just declare) the "struct
tablevalues" type *outside* main(). You can do this at any point
before main(), before or after the prototype for save_in_memory(),
as long as you at least *declare* the existence of the type before
defining the prototype.

Again, keep in mind the difference between a "declaration":

I declare that Santa Claus exists!

which does not actually describe anything, just say that it
exists, and a "definition":

He's the guy formerly known as Joe Shlabotnik. He had his
name legally changed last week.

which not only tells you that something exists, but gives you
all the particulars (which may be quite different from what you
were expecting :-) ).

So now, with all that in mind, we can rewrite the code up to
this point:

#include ... /* do includes, as needed */

struct tablevalues;
[again this just declares that the type exists, so that
save_in_memory can refer back to it]

void save_in_memory(struct tablevalues *data_pointer,
unsigned int *number_of_sets_in_memory,
unsigned int increase_memory_number,
double double_x, int var1, int var2, int var3);

struct tablevalues {
double double_x;
int var1;
int var2;
int var3;
};
[this actually *defines* the type, so now we can use the .var1
member and so on]

int main(void) {
const unsigned int increase_memory_number = 3;
unsigned int number_of_sets_in_memory;
double doub_x;
int va1, va2, va3;

tablevalues *data_pointer;

Since I hate useless "typedef"s, I will just expand this one out:

struct tablevalues *data_pointer;

number_of_sets_in_memory = 0; /* there's nothing stored at
this moment */

/* get user input - and store the data */
printf("Enter a double value, followed by 3 integers:\n");
while( scanf("%lf %d %d %d\n", &doub_x, &va1, &va2, &va3) == 4)
{

[so far so good]

save_in_memory( data_pointer,
&number_of_sets_in_memory,
increase_memory_number,
doub_x, va1,
va2, va3);

Here we have a problem. On the first call to save_in_memory(),
what is the value stored in "data_pointer"? (Did you initialize
it above?) Moreover, you gave save_in_memory() the address of the
thing named "number_of_sets_in_memory", so that save_in_memory()
can change number_of_sets_in_memory, but you did not give
save_in_memory() the address of "data_pointer", so save_in_memory()
*cannot* change "data_pointer".

There are a number of different solutions to this second problem,
at least two of them perfectly reasonable and quite conventional:
either give save_in_memory() the address of data_pointer, or change
it to return a new value for data_pointer, as desired. (A third
solution is to package up several items -- at least the data pointer
and the count of items, and perhaps also the "increase_memory_number",
in yet another "struct" type and pass a pointer to an instance of
that type.)

The rest of the code I am not going to address too much, except
to note serious bugs and mention a technique I find quite useful:

void save_in_memory( struct tablevalues *data_pointer,
unsigned *number_of_sets_in_memory,
unsigned increase_memory_number,
double double_x, int var1,
int var2, int var3)
{

Inside this function, you will refer, quite a few times
(7 times, to be exact) to "*number_of_sets_in_memory". I
find it works better, in such cases, to make a local copy
of *number_of_sets_in_memory, work with the local copy, and
put the final result back "somewhere appropriate" before
returning. ("Somewhere" can be "just at the end", or
"as soon as the new value is known", or whatever is most
convenient.)

unsigned i; /* counter */
["i" is never used]
int *tmp_ptr;

tmp_ptr should have the same type as data_pointer (and if you change
the function to take a pointer to main()'s data_pointer, you can
use the same technique as for *number_of_sets_in_memory here).

/* update counter of how much is saved currently */
*number_of_sets_in_memory++;

The operators "*" and "++" bind ("have precedence", as some put
it, but "bind" is a better word) in a way other than what you want:
this means:

*(number_of_sets_in_memory)++;

but you want:

(*number_of_sets_in_memory)++;

/* need to allocate more memory ? */
if(*number_of_sets_in_memory % increase_memory_number == 0)
{

/* first allocate space for the pointer array */
if ((tmp_ptr = realloc(data_pointer,
(*number_of_sets_in_memory + increase_memory_number)
* sizeof(tablevalues) )) == NULL) {
printf("Memory failure!\n\n");
exit(EXIT_FAILURE);
}
/* ??? possibly I need to copy the old values to the new
array ??? */
data_pointer = tmp_ptr;

You do not need to copy the values (realloc() has done that), but
you *do* need to inform main(), in some way or another, that its
old copy of "data_pointer" is out of date and it should use the
new one.

}

/* Ok, enough memory available - now: store information in
structure-1 */
data_pointer[number_of_sets_in_memory-1]->double_x =
double_x;
data_pointer[number_of_sets_in_memory-1]->var1 = var1;
data_pointer[number_of_sets_in_memory-1]->var2 = var2;
data_pointer[number_of_sets_in_memory-1]->var3 = var3;

If "data_pointer" is a pointer to the structure, then necessarily
"data_pointer[i]" for some valid index i is an instance of the
structure -- so these should be "data_pointer[i].field = value".

}

In this particular case, I might be more likely return the new
data_pointer value from the modified function; but to illustrate
everything I just said, here is a version that takes a pointer to
the "data_pointer" to update, as well as a (separate) pointer to
the "number_of_sets_in_memory" to update:

void save_in_memory(
struct tablevalues **tpp,
unsigned int *nsaved, unsigned int nincrease,
double x, int var1, int var2, int var3
) {
struct tablevalues *tp = *tpp;
int old_n = *nsaved, new_n = old_n + 1;

/* expand the table if needed */
if ((new_n % nincrease) == 0) {
/*
* Note that we have a *copy* of *tpp here. If we trash
* our copy, so what? The original is still in *tpp.
*/
tp = realloc(tp, (old_n + nincrease) * sizeof *tp);
if (tp == NULL) {
/*
* Really should return some failure indication
* rather than just rudely exiting. (As a "low
* level function", we should not be making major
* strategic decisions -- like giving up entirely
* -- at this level: we should just report the
* problem, and let higher level code decide how
* to handle it.) But here save_in_memory() has
* no return value, so no obvious way to indicate
* failure.
*/
fprintf(stderr, "can't allocate memory");
exit(EXIT_FAILURE);
}
*tpp = tp; /* allocation succeeded - update caller's *tpp */
}

/*
* Here old_n is still unchanged, new_n is still old_n+1,
* and either (new_n % nincrease) was nonzero, meaning
* we had room in the table, or the table size has been
* increased successfully, meaning we now have room in
* the table. So just put in the values, and remember
* to update the caller's "number in memory" value too.
*/
tp[old_n].double_x = x;
tp[old_n].var1 = var1;
tp[old_n].var2 = var2;
tp[old_n].var3 = var3;
*nsaved = new_n; /* this can happen anywhere along here */
}

At the moment, the new version is much longer, but only because
of all the comments.
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) +1 801 277 2603
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.
.

Relevant Pages