copying an array of pointers to structures

[jimjim]

Hello,

Any help will be much appreciatted. My problem is as follows:

I declare as global variables:
typedef struct _Node{ ..; ..;}Node;
Node *Graph[MAX];

in a function called initiallise(), I allocate memory and copy information:
for(i )
Graph = (Node*) malloc( sizeof(Node))


in main() I declare:
Node *Queue[MAX]
and I then want to copy whats in Graph (the pointers) in the Queue:
Queue = Graph

so that I can manipulate the Queue (add/remove pointers) but access it like
Queue
which of course doesnt work :-(

Thanks a lot in advance.

 

[Ben Pfaff]

I declare as global variables:
typedef struct _Node{ ..; ..;}Node;

Identifiers that begin with an underscore following by a capital
letter are all reserved. Pick another convention; for example,
you can simply give it Node as the tag.

Node *Graph[MAX];

in a function called initiallise(), I allocate memory and copy information:
for(i )

You'll need more than that inside your parentheses.

Graph = (Node*) malloc( sizeof(Node))

When calling malloc(), I recommend using the sizeof operator on
the object you are allocating, not on the type. For instance,
*don't* write this:

int *x = malloc (128 * sizeof (int)); /* Don't do this! */

Instead, write it this way:

int *x = malloc (128 * sizeof *x);

There's a few reasons to do it this way:

* If you ever change the type that `x' points to, it's not
necessary to change the malloc() call as well.

This is more of a problem in a large program, but it's still
convenient in a small one.

* Taking the size of an object makes writing the statement
less error-prone. You can verify that the sizeof syntax is
correct without having to look at the declaration.

I don't recommend casting the return value of malloc():

* The cast is not required in ANSI C.

* Casting its return value can mask a failure to #include
<stdlib.h>, which leads to undefined behavior.

* If you cast to the wrong type by accident, odd failures can
result.

in main() I declare:
Node *Queue[MAX]
and I then want to copy whats in Graph (the pointers) in the Queue:
Queue = Graph

If you just want to copy the pointers, not the data pointed to,
then use memcpy:
memcpy(Queue, Graph, sizeof Queue);
Be careful: if Queue is actually a function parameter, then this
must be written as
memcpy(Queue, Graph, MAX * sizeof *Queue);
Because the latter form should always work given the declaration
above, a beginner might want to use it consistently instead of
the former.

so that I can manipulate the Queue (add/remove pointers) but access it like
Queue

 

[jimjim]

Ben Pfaff thank you very much for your comments. You were very clear and I
II follow your advice.

I now have a greater problem and have no idea whats wrong :-(


As I said I declare a structure:
typedef in Vertex;
typedef struct Node{
Vertex v;
boolean known;
Distance dist;
Vertex predcssor;
}Node;

then in main() I ve got:
Node *Graph[MAX];

and create structures dynamically:
for(i=1;i<=NUMVERTEX;i++)
Graph= malloc(sizeof (*Graph));

for(i=1;i<=NUMVERTEX;i++)
{
Graph->v = i;
Graph->known = NOTINTREE;
if(atoi(argv[2]) == i)
Graph->dist = 0; //this is the source so set distance to zero
else
Graph->dist = INFINITY; //distance from the source set to INFINITE
Graph->predcssor = NULL; //predecessor in the path set to NULL
}

I run gdb to debug my code. After I allocate memory and get into the second
for loop, for the second time, I find out that there is no member w of the
first struct, ie I write "print Graph[1]->w" and I get there is no member
named w (all this after I m in the loop for the second time, ie data are
entered in the Graph[2] struct. (I also tried Graph= (Mode*)
malloc(sizeof (Node)) for those that didnt read the previous thread.

Please do be verbose in order to understand. Thank you in advance.

 

[Ben Pfaff]

for(i=1;i<=NUMVERTEX;i++)
Graph= malloc(sizeof (*Graph));

Hmm. Based on that code I wonder if I was really as clear as I
could have been. In fact, the proper use of sizeof in that case
would be
Graph= malloc(sizeof (*Graph));
where the innermost parentheses are optional.

Is MAX at least one greater than NUMVERTEX? C arrays run from 0
up to the dimension requested minus 1, so `int x[5];' declares an
array of 5 ints with indexes 0, 1, 2, 3, and 4.

for(i=1;i<=NUMVERTEX;i++)
{
Graph->v = i;
Graph->known = NOTINTREE;
if(atoi(argv[2]) == i)
Graph->dist = 0; //this is the source so set distance to zero
else
Graph->dist = INFINITY; //distance from the source set to INFINITE
Graph->predcssor = NULL; //predecessor in the path set to NULL
}

You could combine this loop with the previous one.

I run gdb to debug my code. After I allocate memory and get into the second
for loop, for the second time, I find out that there is no member w of the
first struct, ie I write "print Graph[1]->w" and I get there is no member
named w

I'm a little baffled why you expect a member named `w', because
your definition for `struct Node' didn't include a member named
`w'.

(all this after I m in the loop for the second time, ie data
are entered in the Graph[2] struct. (I also tried Graph=
(Mode*) malloc(sizeof (Node)) for those that didnt read the
previous thread.

Hmm. Maybe I should have been more clear that sizeof (Node) is
correct, just not the best style.

 

[Chris Torek]

typedef in Vertex;

This is clearly not the code you compiled, because it contains a
typo (the word "in" should read "int"). That makes me suspect that
the rest of what you wrote is also not the code you compiled. As
such, the notes I make here are not necessarily the correct notes.

typedef struct Node{
Vertex v;
boolean known;
Distance dist;
Vertex predcssor;
}Node;

then in main() I ve got:
Node *Graph[MAX];

So far so good (modulo the typo). Note that valid values for
indexing the Graph[] array are 0 through MAX-1 inclusive. If
MAX is (say) 200, Graph[0] is OK, and Graph[199] is OK, but
Graph[200] is out of range.

and create structures dynamically:
for(i=1;i<=NUMVERTEX;i++)
Graph= malloc(sizeof (*Graph));

Is NUMVERTEX different from MAX? If so, why? Note that Graph[0]
is not set here, and if NUMVERTEX is >= MAX, Graph[MAX] is used
when it does not exist. In general, in C, loops over arrays mostly
run "for (i = 0; i < N; i++)", not "for (i = 1; i <= N; i++)".

The call:

Graph = malloc(sizeof (*Graph));

has the wrong general format. Calls to malloc should match the
pattern:

var = malloc(N * sizeof *var); /* or when N == 1: */
var = malloc(sizeof *var);

Here "var" is "Graph" (and N is indeed 1), so this should be:

Graph = malloc(sizeof *Graph)

(The parse order for the expression "*Graph" is the same as that
for *(Graph), so there is no need to parenthesize the variable
name Graph in this case.)

for(i=1;i<=NUMVERTEX;i++)

Again, this loop should probably run from 0 to MAX-1 (although your
algorithm probably depends on no node having index number 0). If
you prefer, you can have Graph[0] be deliberately unused, and change
the declaration to:

Node *Graph[NUMVERTEX + 1]; /* note: Graph[0] is never used */

(assuming NUMVERTEX is a constant).

{
Graph->v = i;
Graph->known = NOTINTREE;

Style comment: Graph->known has type "Boolean" (which is presumably
just "int", unless you are using C99, in which case why not stick with
C99's "bool" in the first place?). "Proper" Boolean values are just
TRUE and FALSE; "NOTINTREE" is neither TRUE nor FALSE.

if(atoi(argv[2]) == i)
Graph->dist = 0; //this is the source so set distance to zero
else
Graph->dist = INFINITY; //distance from the source
set to INFINITE

Usenet-specific: Notice how the "//" comment text has changed, so
that the comment terminates at the word "source". The rest of the
comment has now become an (invalid) line of C code, which will draw
a diagnostic -- probably "syntax error" -- from the compiler. "//"
comments do not work well in code that may be reformatted by Usenet
News systems. "/*" comments do not suffer from this particular
problem.

Style-and-substance: atoi(argv[2]) has to "do work" to calculate
the "int" value of the series of digit characters in argv[2][0],
argv[2][1], argv[2][2], ..., up until argv[2] is '\0'. Unless
the compiler can prove to itself that atoi(argv[2]) returns the
same result on every trip through the loop, the compiler will have
to re-compute that value every time. It would make a lot more
sense to instruct the system to compute it once, and also give it
a name, e.g.:

int source_vertex;

if (argc < 3) {
... /* handle error, not enough arguments */
}
source_vertex = atoi(argv[2]);
/* now you can also check that source_vertex is a sensible number */
...
for (...) {
Graph->v = i;
Graph->known = FALSE; /* or whatever */
/* the source vertex is at distance 0; others are at INFINITY
until later calculation says otherwise */
Graph->dist = i == source_vertex ? 0 : INFINITY;

Graph->predcssor = NULL; //predecessor in the path set to NULL
}

According to the "struct Node" definition, the "predcssor" field
has type Vertex, which is an alias for "int" (assuming the typo
correction I made was the right one). NULL is not a suitable
value for "int"s, as it may -- at the C compiler's discretion --
be defined as ((void *)0).

If you are using the "Graph[0] is never used so that I can use
index 0 to mean no index" trick (as I suspect you are), and if
you really do mean to have the "predcssor" field contain the
index number of the predecessor (as opposed to, say, a value of
type "pointer to struct Node" pointing to the same place Graph[x]
would point to for predecessor node #x), just set it to 0.
If you *do* want predcssor to have type "struct Node *", give it
that type.

I run gdb to debug my code. After I allocate memory and get into the second
for loop, for the second time, I find out that there is no member w of the
first struct, ie I write "print Graph[1]->w" and I get there is no member
named w ...

There is indeed no member named "w". There are four members,
named "v", "known", "dist", and "predcssor".

 

[Barry Schwarz]

Ben Pfaff thank you very much for your comments. You were very clear and I
II follow your advice.

I now have a greater problem and have no idea whats wrong :-(


As I said I declare a structure:
typedef in Vertex;
typedef struct Node{
Vertex v;
boolean known;
Distance dist;
Vertex predcssor;
}Node;

then in main() I ve got:
Node *Graph[MAX];

and create structures dynamically:
for(i=1;i<=NUMVERTEX;i++)
Graph= malloc(sizeof (*Graph));

for(i=1;i<=NUMVERTEX;i++)
{
Graph->v = i;
Graph->known = NOTINTREE;
if(atoi(argv[2]) == i)
Graph->dist = 0; //this is the source so set distance to zero
else
Graph->dist = INFINITY; //distance from the source set to INFINITE
Graph->predcssor = NULL; //predecessor in the path set to NULL
}

I run gdb to debug my code. After I allocate memory and get into the second
for loop, for the second time, I find out that there is no member w of the
first struct, ie I write "print Graph[1]->w" and I get there is no member
named w (all this after I m in the loop for the second time, ie data are

Where did you think the w was going to come from. Each Graph
points to a Node (which is an alias for struct Node). This structure
has four members: v, known, dist, and predcssor. Not a w in sight.

entered in the Graph[2] struct. (I also tried Graph= (Mode*)
malloc(sizeof (Node)) for those that didnt read the previous thread.

Please do be verbose in order to understand. Thank you in advance.

<<Remove the del for email>>