Thread: Linked list error - sample faulty

I'm having a tough time understanding how to use linked lists as the tutorial is unclear. I copied the sample code, but I'm getting compiler errors in areas involving malloc. This is that tutorial I'm referring to.

I copied the sample code, but when it reaches the line with "malloc", it errors saying of it being unable to "convert void * to node *".

Code:

void linkedListTest()
{
	struct node *root;
	struct node *conductor;
	root = malloc(sizeof(struct root)); // 2 errors!
	root->next;
	root->x = 12;
	conductor = root;
	
	if (conductor != 0)
	{
		while (conductor->next != 0)
		{
			conductor = conductor->next;
		}
	}

	conductor->next = malloc(sizeof(struct node)); // errored

	if (conductor == 0)
	{
		printf("Out of memory!");
		return;
	}

	conductor->next = 0;
	conductor->x = 0;
}

The function is called in another function. The 3 errors I get are these:

.\firsttest.cpp(29) : error C2027: use of undefined type 'linkedListTest::root'
.\firsttest.cpp(29) : see declaration of 'linkedListTest::root'
.\firsttest.cpp(29) : error C2440: '=' : cannot convert from 'void *' to 'node *'
Conversion from 'void*' to pointer to non-'void' requires an explicit cast
.\firsttest.cpp(42) : error C2440: '=' : cannot convert from 'void *' to 'node *'
Conversion from 'void*' to pointer to non-'void' requires an explicit cast

I'm using Visual C++ 2005 Express. My intention after learning the basics is to write a series of up to 2 1/4 million 8-bit values to be used as a buffer for displaying images.

Edit: The tutorial also doesn't clearly mention how to add more items to the list and set their values. That is, I want 32, 96, 128, 48, 240, and so on.

the compiler won't compile this because you have a potential error. The way to tell the compiler you know what you are doing is to change this
conductor->next = malloc(sizeof(struct node)); // errored
to

Code:

conductor->next = (node *)malloc(sizeof(struct node)); // errored

the (node *) say's that the data that is providing is a pointer to a node. This needs to be done to help prevent bugs in programs.

Similarly to the line

Code:

root = malloc(sizeof(struct root)); // 2 errors!

Looking at the code it doesn't seem to make sense. Here is how I changed it and tryed to make sense of it

Code:

// I changed the name to what it looks like the original code is trying to do
void add_node_to_end( struct node *root)
{
	// struct node *root;
	struct node *conductor;
	// root = malloc(sizeof(struct root)); // really this line doesn't make sense interms of purpose
	// root->next;
	// root->x = 12; editing its own contents? I don't understand why
	conductor = root;
	
	if (conductor != 0)
	{
		while (conductor->next != 0)
		{
			conductor = conductor->next;
		}
	}

	conductor->next = (node *)malloc(sizeof(struct node)); // errored

	if (conductor == 0)
	{
		printf("Out of memory!");
		return;
	}
	 //  origonally it allocats memory and then sets next to 0
	// resulting in a memory leak
	conductor = conductor->next
	conductor->next = 0;
	conductor->x = 0;
}

Hope this helps

> the (node *) say's that the data that is providing is a pointer to a node. This needs to be done to help prevent bugs in programs.

If <stdlib.h> is included as it should be whenever using malloc(), the cast isn't necessary - in fact, it makes errors MORE likely, since if you cast explicitly you may do it wrong. If you let the cast happen automatically then you're guaranteed to get the right one.

Edit: Although if the cast is wrong, the compiler will complain, so it's not serious. Still, generally speaking, anything that increases the volume of code tends to make it buggier, and using explicit casts after every malloc() significantly bulks up the code.

That very code is posted in that tutorial I pointed to, at cprogramming.com. Although I originally didn't have the "stdlib.h" thing included, I included it. These errors are different from the ones I got without it. There seems to be a serious flaw with that sample code and it should be fixed.

Edit: As a side note, I'm first after creating the buffer. 640x480 resolution would mean 921600 bytes (24-bit color) are needed or 921600 items in the list. Due to needing double buffers (one for drawing, the other writing data into (changing), then swapping the two). Afterwards, the data in the buffer is altered by changing the values one at a time. First, however, I'm just after the basics which is why I copied that code in the tutorial.

I imagine you're trying to compile it as a C++ project instead of a C project.

If that is the case, then a cast must be done when using malloc(), and this should be in the C++ section of the forums anyway.

How ironic that this subject has come up.

Originally Posted by Benzakhar

The way I think about it haveing to be forced to cast helps to remind the types of the variables. Personally I prefer to be forced to cast as I don't usually want things automatically converted or converted to other types.

After spending some time thinking about it, I've concluded that the sole purpose of the cast is to act as a comment. It doesn't affect correctness, since if the pointer type one is assigning to is correct, then the compiler will require an explicit cast to be correct, otherwise compile will fail. And if the pointer type is wrong, the cast won't help.
As far as being used as a comment goes, first, one could just use a regular comment instead. Second, a typical malloc() already acts as its own comment, since it looks something like

Code:

  p = malloc(400*sizeof(int));

and the type in the sizeof() tells you what the pointer type is (int * in this case). Of course, for this to work consistently, one has to always use the exact type in sizeof() - for example, if p is a pointer to unsigned int, one would have to write

Code:

  p = malloc(400*sizeof(unsigned int));

even though signed and unsigned int have the same size. And if p is a pointer to char, then one would have to explicitly write sizeof(char), even though this is always 1 (same goes for signed char and unsigned char), and I know some people hate to see that. Thirdly, using the explicit casts everywhere makes the source code bigger and makes it harder to spot bugs. Using the argument to sizeof() for the same purpose takes up less space since most malloc()s have the correct sizeof() type already. So to sum up, I think a better way to get the comment effect would be to put the exact type inside the sizeof() inside the malloc(), and to do it consistently.

Edit: Of course, doing this prevents the common trick of writing

Code:

  p = malloc(400*sizeof(*p));

so one can choose either to get the comment effect by putting an explicit type inside sizeof(), or to be able to change types easily by giving it up.

Edit: And of course it makes no sense to do something like

Code:

  p = (int *)malloc(400*sizeof(*p));

since the whole point of using sizeof(*p) is to allow having the expression work automatically when the type is changed.