Thread: Passing a pointer as an argument

Hi all,
I am having trouble modifying a linked list. I am writing a function to delete the last node from the linked list, but it gave me incompatible types error.
Here is my struct:

Code:

typedef struct PCB{
    int id;
    struct PCB *next;
    struct PCB *prev;
}PCB_rec, *PCB_p;

Here is my function to delete the last node (given the pointer is pointing at the last node of the list):

Code:

void del_last_node(PCB_p *process_list){
        PCB_p temp = process_list;
        if (temp->prev != NULL){
                temp = temp->prev;
                temp->next->prev = NULL;
                temp->next = NULL;
        }
        else {
                temp = NULL;
        }
        process_list = temp;
}

And here is how I called the function:

Code:

del_last_node(&process_list);

It gives me the following errors:
initialization from incompatible pointer type at line:
PCB_p temp = process_list

assignment from incompatible pointer type at line:
process_list = temp

Can you guys give me some hints at what I did wrong? Thanks in advance.

I've tried to stay away from typedefing structs as pointers (it's all over the place in the Windows API), so I could be wrong here, but it looks to me like PCB_p temp is actually a sort of hidden pointer, so it's really PCB_p* temp. process_list is of type PCB_p* too, but since it's passed into the function with it's address, inside the function it's actually PCB_p** process_list. So a one level pointer is trying to have a two level pointer assigned to it.

As I said, I've tried to avoid typedef and pointers because it seems to me like it basically hides the fact that it's a pointer, but given my lack of experience with that, I could be wrong!

Originally Posted by effup

Thank you for the prompt reply. So you suggested that I should have:
PCB_p *temp = process_list
instead of
PCB_p temp = process_list ?
Thanks again. I really appreciate it
Edit: That gives me even more errors ( thanks though

See, that's exactly why I:

Originally Posted by AndrewHunter

Don't typedef pointers!!!!!!!

as he says.

I actually tried compiling the code this time and found the problem, but do as Andrew suggests and see what happens.

Originally Posted by Sorin

See, that's exactly why I:



as he says.

I actually tried compiling the code this time and found the problem, but do as Andrew suggests and see what happens.

Thank you for your input guys. I'm pretty new to C. I will try to see what the problem is per you guys' suggestions

So I changed it to

Code:

void del_last_node(PCB_p process_list){
          PCB_p temp = process_list;
}

It compiles without any error now, but instead of doing what I want, which is deleting the last node, it deletes everything else but that last node
Sorry I could not go back to remove typedefs and change my code since it requires reading inputs and I have written quite a bit of code

I am sorry for all the issues in my code that you mentioned. Here is what I have now with your suggestion:

Code:

void del_last_node(PCB_p process_list){
        PCB_p temp = process_list;              // Set temp to the node that will be deleted
        if (temp->prev != NULL){                // If this is NOT the last process
                PCB_p temp2 = temp;             // temp2 is the node to be deleted now
                temp = temp->prev;          // Move back one node
                temp->next->prev = NULL;    // Set the link to the previous node of the node to be deleted to NULL
                temp->next = NULL;              // Remove the link from the "new" last node to the delted node
                free(temp2);                    // Free that deleted node
        }
        else {
                free(temp);
                //temp = NULL;
        }
        process_list = temp;
}

Does it look ok now?

I see now. To use a void function to delete a node, you need a pointer to a pointer. Since I have typedef my pointer, my function would be:

Code:

void DeleteNode(PCB_p* root, PCB_p* nodeToDelete)
{
    if ((nodeToDelete)->prev)
    {
        (nodeToDelete)->prev->next = (nodeToDelete)->next;
        (nodeToDelete)->next->prev = (nodeToDelete)->prev;
    }
    else
    {
root = (nodeToDelete)->next;
root->prev = NULL;
    }
    free(nodeToDelete);
}

Is that right? And I think this will gives me an error if I try to delete the last node right here:

Code:

(nodeToDelete)->next->prev = (nodeToDelete)->prev;

Since there is no (nodeToDelete)->next (points to NULL). Is it right? Or am I missing something? Thank you for the time and your help though. I really appreciate it

Sorry it's been 3 weeks. I managed to get around that by giving it a return type instead of returning nothing (void). I still can't figure it out using void() function though. Can you help me out? Let's just say I wanna make a void function that will modify the value of a node's id

effup, consider this trivial example:

Code:

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

typedef struct node  node;
struct node {
    struct node *next;
    int           value;
};

void prepend_incorrect(node *list, node *item)
{
    item->next = list;
    list = item;
}

void prepend(node **listptr, node *item)
{
    item->next = *listptr;
    (*listptr) = item;
}

In C, pointers are passed by value. This means that any changes made to the variables list, listptr, and item in the above two functions are only visible within the functions themselves; they are not propagated to the caller.

In the prepend_incorrect function, the node pointed to by item will have its next field modified to point to list , but the caller will not see any changes to list.

To propagate the change to the caller, we need to get the pointer to the pointer to the first element. Any changes we might do to listptr will still be only visible within the caller, but that's okay: the first element in the list is *listptr, and if we modify it, the caller will see it too.

Here is an example main to show how to use the above prepend function. This one takes one or more integers as command line parameters, prepends each to mylist, then prints out the structure of the mylist in DOT format:

Code:

int main(int argc, char *argv[])
{
    node *mylist = NULL;
    node *curr;
    int   value, arg;
    char  dummy;

    if (argc < 2) {
        fprintf(stderr, "\nUsage: %s NUMBER [ NUMBER ... ]\n\n", argv[0]);
        return EXIT_FAILURE;
    }

    for (arg = 1; arg < argc; arg++) {
        if (sscanf(argv[arg], " %d %c", &value, &dummy) == 1) {

            curr = malloc(sizeof *curr);
            if (curr == NULL) {
                fprintf(stderr, "Out of memory.\n");
                return EXIT_FAILURE;
            }

            curr->next = NULL;
            curr->value = value;
            prepend(&mylist, curr);

        } else {
            fprintf(stderr, "%s: Not an integer.\n", argv[arg]);
            return EXIT_FAILURE;
        }
    }

    printf("digraph {\n");
    printf("\trankdir=\"LR\"; \n");
    for (curr = mylist; curr != NULL; curr = curr->next) {
        printf("\t\"%p\" [ label=\"%d\" ];\n", curr, curr->value);
        printf("\t\"%p\" -> \"%p\";\n", curr, curr->next);
    }
    printf("\t\"%p\" [ label=\"NULL\", shape=none ];\n", NULL);
    printf("}\n");

    return EXIT_SUCCESS;
}

First, you'll need Graphviz. It's free, and available for all major platforms (Linux, Windows, Mac). If using Linux, use your package manager to install it; it should be available in the standard software sources for your distribution, no need to go to that website at all (other than read the documentation, maybe).

In Linux, you can compile and run the above (combined into say example.c) using

Code:

    gcc -Wall -O2 example.c -o example
    ./example 1 2 3 4 5 | dot -Tx11

In Windows, compile the example to example.exe, then run

Code:

    example 1 2 3 4 5 | dot -Tpng > example.png

and open the generated graph, example.png, in a browser. (I like the svg format much better, but not being a Windows user, I don't know if it is yet supported well enough in Windows.)

It'll look like this:

Of course this example is trivial, each number being prepended to the list one at a time, producing the original list in reverse order.

The DOT language output looks like this:

Code:

digraph {
	rankdir="LR"; 
	"0x60e090" [ label="5" ];
	"0x60e090" -> "0x60e070";
	"0x60e070" [ label="4" ];
	"0x60e070" -> "0x60e050";
	"0x60e050" [ label="3" ];
	"0x60e050" -> "0x60e030";
	"0x60e030" [ label="2" ];
	"0x60e030" -> "0x60e010";
	"0x60e010" [ label="1" ];
	"0x60e010" -> "(nil)";
	"(nil)" [ label="NULL", shape=none ];
}

so it's a very easy graph description language.

My sneaky reason for posting this message, you see, was to also show how to debug and visualize any pointer-based data structures.

I can't even count how many issues I've solved by printing the DOT format description for a structure before and after some problematic operation (single function call). It's so much easier to exactly see what has happened that way -- and so much easier to discover the bug, too.

Of course, if you typedef your pointer types, all of the above gets very hairy to read, and quite difficult to get right.. which is why I, too, never typedef my pointer types.

For more complicated data structures, Graphviz has other visualizers you can try in addition to dot: neato, twopi, circo, and fdp.