C pointers

Kevo

Hey,
Im trying to write a linkedlist in c. I cant find any logic errors in my prog but it just won't work. I keep getting segmentation faults.

I'm pretty sure the following code is correct but I just want to make sure.
Will the following code assign the pointer(successor) to the address of newNode?

Node->successor = (void *)&newNode;

Thanks

ianhobo

what type is successor?
and why are you casting new node to a void? (To match the type of successor?)
I imagine it should be type Node? assuming your structure is type Node?

Kevo

Thanks for the reply.
Successor is a pointer to a node. It's declared in a struct.

struct node *successor;

I have tried casting it to a node:

thisNode->succ = (node *)&newNode;

but I get a warning:
"assignment from incompatible pointer type"

I dont understand why I am getting this warning.

ianhobo

can you post your code? specifically your node structure

if newNode is declared as type node, and successor is type node, then you shouldn't need to cast anything

if I don't reply, sorry, im heading into a meeting at half!

off the top of my head, try something like this:

typedef struct
{
  Node *next;
}Node;

void main(void)
{
    Node *myNode1;
    Node *myNode2;

   myNode1->next = myNode2;
}

sorry, i dont have time to check it.........

Kevo

This is my struct.

typedef struct
{
	struct node *successor;
} node;

node *nodes;
node *nodes2;


after the nodes are declared and initialised I try to link them:
nodes->successor = (void*)&nodes2;//This gives no error but it doesnt do waht I want it to.
The pointers end up pointing to an incorrect memory location.

Heres an example of the output:



The first pointer points to: 804a008

The second pointer points to: 804a068
The firstNode successor points to(This must match the previous): bfe9a09c - does not match

The third pointer points to: 804a0c8
The firstNode successor points to(This must match the previous): bfe9a0a4 - does not match

Heres a copy of the print statement I used:
printf("The second pointer value is: %x\n", nodes2);
printf("This must match the previous: %x\n\n", nodes->succ);


I have tried this in many ways(some of which are definitely wrong):

nodes->succ = (node*)&nodes2;
error: assignment from incompatible pointer type


nodes->successor = (node*)nodes2;
error: conversion to non-scalar type requested

nodes->successor = (node)nodes2;
error: conversion to non-scalar type requested

nodes->successor = nodes2;
error: assignment from incompatible pointer type

Kevo

Your way seems to be working. Turns out it wasn't an eror. just a warning.

warning: assignment from incompatible pointer type

Is it ok for this to happen?

Tobias Greeshman

Ok first of all what compiler are you using?

You shouldn't need to cast anything in your code relating to pointers (well the code you've posted anyways).

//This gives no error but it doesnt do waht I want it to.
nodes->successor = (void*)&nodes2;

You've essentially made successor a void pointer, you've told the compiler "I don't know what type of data this points to", so you can't do any pointer arithmetic. Its not giving you any warnings because its perfectly legal in C to do this, but the compiler assumes you know what you're doing.

Then the errors you were getting were all due to trying to point void pointer to the node data, and the compiler was unable to do the conversion.

Any of these lines would compile if changed to something like

(Node*)(nodes->successor) = nodes2;

As a general rule of thumb never cast anything to void* unless its required. It's perfectly good practice to begin with a void* pointer and cast to another type, the best example being the "malloc" function.

carveone

Anytime you see a
"warning: assignment from incompatible pointer type"
treat that as an error. Generally it means you've done something like:
int *ip;
ip = 5;
Although, probably more subtle than that...

Kevo wrote: »

This is my struct.

typedef struct
{
	struct node *successor;
} node;

node *nodes;
node *nodes2;

Um. I assume there's some typos in there and you mean

struct node
{
struct node * successor;
};

typedef struct node node_t;

Or similar. In C you can have the same name for the typedef as for the structure (different namespaces) so I'll assume that
ie:
typedef struct node node;

after the nodes are declared and initialised I try to link them:
nodes->successor = (void*)&nodes2;//This gives no error but it doesnt do waht I want it to.

nodes->successor : is a pointer to a struct node
&nodes2 : is the address of a pointer to a struct node.

Mismatch. which you have forced down the compiler's throat by putting the (void *) in (never do this)...

The pointers end up pointing to an incorrect memory location.

I'd imagine they would! You are guessing...

nodes->successor = nodes2;
error: assignment from incompatible pointer type

That one is right. Something is missing you aren't telling us about!

I've this feeling that you aren't assigning storage either. "node *nodes" makes a pointer to a node type. Pointing where exactly? It will be initialised to garbage, so taking some random memory location, making it look like a structure and playing with it is not the right approach! You will get a segment violation.

Cheers,

Conor.

carveone

silas wrote: »

You shouldn't need to cast anything in your code relating to pointers (well the code you've posted anyways).

Absolutely! Casting pointers is generally the first sign that something is wrong.

Any of these lines would compile if changed to something like

(Node*)(nodes->successor) = nodes2;

One cannot cast an Lvalue but I know what you mean...

C is not C++. A typedef is an alias not a type in its own right. The (Node *) should not be necessary.
And no example I've seen has allocated any storage!

Conor.

Kevo

Thanks for your help.
Got it working. I completely restructured my program and it doesn't seem to be having any problems. I'm not sure what was causing the warnings but they're gone now.

Tobias Greeshman

carveone wrote: »

One cannot cast an Lvalue but I know what you mean..

I think you might actually be correct here, I haven't the energy to dredge through the C++ spec to see if this is allowed or not. But I do know that it was permitted in GCC, up until 4.0.1. I believe a few other older compilers permitted it also.

I'd imagine though that it may have undefined behaviour.

What I was stating was that the original cast to a void pointer, was causing all the problems, and if it was cast back to its original type, then none of the error/warning messages would of arose.

carveone

silas wrote: »

I think you might actually be correct here, I haven't the energy to dredge through the C++ spec to see if this is allowed or not. But I do know that it was permitted in GCC, up until 4.0.1. I believe a few other older compilers permitted it also.

Permitted probably by accident I'd say... The C faq , question 4.5 has more to say on the subject so I won't!

What I was stating was that the original cast to a void pointer, was causing all the problems, and if it was cast back to its original type, then none of the error/warning messages would of arose.

Yes, it happens all too often when people don't understand the warning and proceed to muffle the compiler. You can go:

int *i;
int k;
...
i = k;

and when you get the warning, proceed to cast the k to an (int *), but bad things will probably happen! What was probably meant was
i = &k;
but instead the programmer beats the compiler around the head with a cast.

I see it a lot in Windows programs where LPTSTRs get cast to bizzarre things (mostly void *) in an attempt to get the compiler to stop bitching :rolleyes:

Conor.