Thread: Fork(), pause(), and storing PID's in a linked list

  1. #1
    Registered User
    Join Date
    Apr 2007
    Posts
    7

    Fork(), pause(), and storing PID's in a linked list

    Hey everyone,

    I've been beating myself up over this one, so any help would be amazing. First, let me give you some background information. I am limited to using the calls fork, pause, kill, malloc, and free. With that in mind, my goal here is to determine what is the maximum number of simultaneous processes that Minix will support for a single user. That said, I need to 1) Fork until I can no longer fork, and pause the process every time I fork so that I can kill it later. 2) Keep track of the pid's in a linked list. Here's my problem, it seems that I can't get nodes into the linked list properly. I really have no clue what to do.

    Here's my code:
    Code:
    #include <sys/types.h>
    #include <stdlib.h>
    #include <alloca.h>
    #include <stdio.h>
    #include <unistd.h>
    
    struct pnode {
    	int pid;
    	struct pnode *next;
    };
    typedef struct pnode pnode;
    
    
    int
    main(void)
    {
    	void startKilling(pnode *list);
    	void createprocesses(pnode *list, pnode *current, pnode **temp);	
    	void getnode(pnode *ptr);
    	
    	pnode *list;
    	pnode *current;
    	pnode *temp1;
    
    	int temp;
    	
    	pid_t pID;	
    	pID = 0;
    	
    	current = list;
    	
    	createprocesses(list, current, &temp1);
    	printf("PID1:  %d\n", list->pid);
    	printf("PID2:  %d\n", list->next->pid);
    
    	exit(0);
    }
    
    void getnode(pnode *ptr)
    {
    	ptr = malloc(sizeof(ptr));
    	printf("Address of ptr: %d\n", &ptr);
    }
    
    void insertnode(pnode *current, pnode *temp, int processid)
    {
    	pnode temp2;
    	getnode(&temp2);
    	printf("Current address: %d\n", &current);
    	printf("Address: %d\n",&temp2);
    	current->pid = processid;
    	current->next = &temp2;
    	*(&current) = &temp2;
    	printf("Current address after reassign: %d\n", &current);
    }
    
    void startKilling(pnode *list)
    {	
    	printf("Merrily killing processes....\n");
    	printf("Anything after this should not exist.\n");
    }
    
    void createprocesses(pnode *list, pnode *current, pnode **temp) {
    	void insertnode(pnode *current, pnode *temp, int processID);
    	
    	pid_t pID;
    	int *counter;
    
    	pID = 0;
    	counter = 0;
    	
    	while(pID != -1) {
    		pID = fork();
    		if(pID == 0) {
    			pause();
    			counter = counter + 1;
    		} else if(pID < 0) {
    			printf("Failed to fork...\n");
    		} else {
    			/* Parent Process */
    			printf("Process ID: %d\n", pID);
    			insertnode(current, (*temp), pID);
    		}
    	}
    	printf("Counter: %d\n", counter);
    }
    As you can tell by looking at my code, I've tried many different methods of doing this, but none of them worked. It seems that whenever I insert a node into the list, it simply overwrites the previous one (in this case, the node pointed to my *list). So basically I never actually insert anything, but simply just keep overwriting *list.

    Thanks ahead of time. I'm just not having any luck with it.

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    You need to allocate memory for the "current" node.

    This is wrong. (Yes, it compiles, but it's not doing anything useful)
    Code:
    *(&current) = &temp2;
    I'm pretty sure you are passing around a few too many variants of pnode at times, but bet that as it may, your insertNode should look like this:
    Code:
    void insertnode(pnode **current, int processid)
    {
    	pnode temp2;
    	getnode(&temp2);
    	current->pid = processid;
    	current->next = temp2;
    	*current = temp2;
    }
    Your other functions may have to change accordingly.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Registered User
    Join Date
    Sep 2007
    Posts
    9
    Quote Originally Posted by matsp View Post
    You need to allocate memory for the "current" node.

    This is wrong. (Yes, it compiles, but it's not doing anything useful)
    Code:
    *(&current) = &temp2;
    I'm pretty sure you are passing around a few too many variants of pnode at times, but bet that as it may, your insertNode should look like this:
    Code:
    void insertnode(pnode **current, int processid)
    {
    	pnode temp2;
    	getnode(&temp2);
    	current->pid = processid;
    	current->next = temp2;
    	*current = temp2;
    }
    Your other functions may have to change accordingly.

    --
    Mats
    yep.............it looks reasonable.....................but implementation could be more simpler
    Last edited by PrashantVaidwan; 09-26-2007 at 03:28 AM.

  4. #4
    Registered User
    Join Date
    Apr 2007
    Posts
    7

    No luck...

    I've implemented that changes that were pointed out to me, fixed any compilation errors, but I still get the same problem. My code now looks like:
    Code:
    #include <sys/types.h>
    #include <stdlib.h>
    #include <alloca.h>
    #include <stdio.h>
    #include <unistd.h>
    
    struct pnode {
    	int pid;
    	struct pnode *next;
    };
    typedef struct pnode pnode;
    
    
    int
    main(void)
    {
    	void startKilling(pnode *list);
    	void createprocesses(pnode *list, pnode *current, pnode **temp);	
    	void getnode(pnode *ptr);
    	
    	pnode *list;
    	pnode *current;
    	pnode *temp1;
    
    	int temp;
    	
    	pid_t pID;	
    	pID = 0;
    	
    	current = list;
    	
    	createprocesses(list, current, &temp1);
    	printf("PID1:  %d\n", list->pid);
    	printf("PID2:  %d\n", list->next->pid);
    
    	exit(0);
    }
    
    void getnode(pnode *ptr)
    {
    	ptr = malloc(sizeof(ptr));
    	printf("Address of ptr: %d\n", &ptr);
    }
    
    void insertnode(pnode **current, int processid)
    {
    	pnode temp2;
    	getnode(&temp2);
    	(*current)->pid = processid;
    	(*current)->next = &temp2;
    	(*current) = &temp2;
    }
    
    void startKilling(pnode *list)
    {	
    	printf("Merrily killing processes....\n");
    	printf("Anything after this should not exist.\n");
    }
    
    void createprocesses(pnode *list, pnode **current, pnode **temp) {
    	void insertnode(pnode **current, int processID);
    	
    	pid_t pID;
    	int *counter;
    
    	pID = 0;
    	counter = 0;
    	
    	while(pID != -1) {
    		pID = fork();
    		if(pID == 0) {
    			pause();
    			counter = counter + 1;
    		} else if(pID < 0) {
    			printf("Failed to fork...\n");
    		} else {
    			/* Parent Process */
    			printf("Process ID: %d\n", pID);
    			
    			insertnode(current, pID);
    		}
    	}
    	printf("Counter: %d\n", counter);
    }
    Perhaps I missed something small?

  5. #5
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
    void insertnode(pnode **current, int processid)
    {
    	pnode temp2;
    	getnode(&temp2);
    	(*current)->pid = processid;
    	(*current)->next = &temp2;
    	(*current) = &temp2;
    }
    That's still not right. Need to be something like this:
    Code:
    void getnode(pnode **ptr)
    {
    	*ptr = malloc(sizeof(**ptr));
    	printf("Address of ptr: %p\n", ptr);
    }
    
    void insertnode(pnode **current, int processid)
    {
    	pnode *temp2;
    	getnode(&temp2);
    	(*current)->pid = processid;
    	(*current)->next = temp2;
    	(*current) = temp2;
    }
    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  6. #6
    Registered User
    Join Date
    Apr 2007
    Posts
    7

    Nope..

    So I implemented the changes seen in your previous post. However, they had no effect. Back in main() when I try to do
    Code:
    printf("PID1:  %d\n", list->pid);
    printf("PID2: %d\n", list->next->pid);
    I get a valid value for PID1, and then garbage for PID2(it never prints [assuming because it points to nothing]).

    My output is this:
    Process ID: 99
    Address of ptr: 0x202d8
    Process ID: 100
    Address of ptr: 0x202d8
    Process ID: 101
    Address of ptr: 0x202d8
    Process ID: 102
    Address of ptr: 0x202d8
    Process ID: 103
    Address of ptr: 0x202d8
    Process ID: 104
    Address of ptr: 0x202d8
    Process ID: 105
    Address of ptr: 0x202d8
    Process ID: 106
    Address of ptr: 0x202d8
    Process ID: 107
    Address of ptr: 0x202d8
    Process ID: 108
    Address of ptr: 0x202d8
    Process ID: 109
    Address of ptr: 0x202d8
    Process ID: 110
    Address of ptr: 0x202d8
    Process ID: 111
    Address of ptr: 0x202d8
    Process ID: 112
    Address of ptr: 0x202d8
    Failed to fork...
    Counter: 0
    PID1: 2056
    l

  7. #7
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    But "List" is a different variable than your list, which with the current design is held by "Current".

    It may be prudent to check if "list->next" is not NULL too...

    Edit: Just remove the "current" in main, remove "list" from the CreateProcess, and use "&list" instead of "&current", and it should work fine.

    Of course, there are other problems in your code: counter in "createprocess" is a pointer, not an integer. It will not work as you expect.

    And I still reccon that you aren't using -Wall (or you are ignoring the warnings), because it should give you at least half a dozen or more warnings the way you are mixing and matching different pointers in various places.

    --
    Mats
    Last edited by matsp; 09-26-2007 at 07:51 AM.
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  8. #8
    Registered User
    Join Date
    Apr 2007
    Posts
    7

    Cleaned up

    After reading your last reply, I decided to go through and try something a bit different. It won't work, but I feel I'm much closer than previously.
    Code:
    #include <sys/types.h>
    #include <stdlib.h>
    #include <alloca.h>
    #include <stdio.h>
    #include <unistd.h>
    
    struct pnode {
    	int pid;
    	struct pnode *next;
    };
    typedef struct pnode pnode;
    
    
    int
    main(void)
    {
    	void startKilling(pnode *list);
    	void createprocesses(pnode *current);	
    	void getnode(pnode *ptr);
    	void insertnode(pnode *current, int processID);
    	
    	pnode *list;
    	pnode *current;
    
    	int temp;
    	
    	pid_t pID;	
    	pID = 0;
    
    	getnode(current);
    	getnode(list);
    	
    	current = list;
    	
    	createprocesses(current);
    
    	printf("List->pid: %d\n", current->pid);
    	current = current->next;
    	printf("List->pid: %d\n", current->pid);
    	exit(0);
    }
    
    void getnode(pnode **ptr)
    {
    	*ptr = malloc(sizeof(**ptr));
    }
    
    void insertnode(pnode *current, int processID)
    {
    	pnode *temp;
            getnode(&temp);
    	current->pid = processID;
    	current->next = temp;
    	current = temp;
    }
    
    void startKilling(pnode *list)
    {	
    	printf("Merrily killing processes....\n");
    	printf("Anything after this should not exist.\n");
    }
    
    void createprocesses(pnode **current) {
    	
    	pid_t pID;
    	
    	while(pID != -1) {
    		pID = fork();
    		if(pID == 0) {
    			pause();
    		} else if(pID < 0) {
    			printf("Failed to fork...\n");
    		} else {
    			/* Parent Process */
    			printf("Process ID: %d\n", pID);
    			insertnode((*current), pID);
    		}
    	}
    }
    Maybe if you know my thought process it will help to clarify some things.
    1) In main I declare a node called *list and *current. *list will always represent the first node in linked list. *current should always represent an empty "NULL" node.
    2) In main, I call getnode() on both list an current to give them memory. This should also give them seperate addresses.
    3) I call createprocesses from main, passing list into it. Once I've forked as many times as possible, I start calling insertnode, and passing it *current and pid.
    4) Inside of insertnode, I create a temp node and then allocate memory towards it. I then set the current->pid, the current->next to temp. Then I set current = next.

    Logically I feel that this should work. If my logic is wrong or I'm using pointers wrong, let me know. I really don't know what else to do.

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Ok, one of the fundamental principles in C is that "If you want to modify something OUTSIDE a function with code IN a function, you need to have a pointer to the object you want to modify."

    Thus:
    Code:
    // This will not change x outside the function
    void func1(int x)
    {
       x = 7;
    }
    
    // this WILL change x outside the funciton
    void func2(int *x)
    {
       *x = 7;
    }
    
    int main() {
       int x = 3;
       func1(x);
       printf("x = &#37;d\n", x);
       func2(&x);
       printf("x = %d\n", x);
       return 0;
    }
    The same obviously applies when you need to modify a pointer. E.g.

    Code:
    // Allocate some memory 
    int pfunc1(int *p)
    {
        p = NULL;
    }
    
    int pfunc2(int **p)
    {
        *p = NULL;
    }
    
    int main()
    {
       int *p = malloc(10);
       printf("p = %p\n", p);
       pfunc1(p);
       printf("p = %p\n", p);
       pfunc2(&p);
       printf("p = %p\n", p);
       return 0;
    }
    Does this explain why you are still not setting your current & list variables OUTSIDE the function where you actually link your list up?

    --
    Mats
    Last edited by matsp; 09-26-2007 at 01:11 PM. Reason: Fix missing & sign
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  10. #10
    Registered User
    Join Date
    Apr 2007
    Posts
    7

    Okay, I think I get it...

    First of all, thank you so much for being patient. Anyways, I read what you said about pointers. I already knew this, but it seems I was getting some fundamentals mixed up. I've implemented changes that I think should work, but it gets a bit messy in the insertnode function, and also when I call it. Here are the changes.

    Code:
    #include <sys/types.h>
    #include <stdlib.h>
    #include <alloca.h>
    #include <stdio.h>
    #include <unistd.h>
    
    struct pnode {
    	int pid;
    	struct pnode *next;
    };
    typedef struct pnode pnode;
    
    
    int
    main(void)
    {
    	void startKilling(pnode *list);
    	void createprocesses(pnode *list, pnode **current, pnode **temp);	
    	void getnode(pnode **ptr);
    	
    	pnode *list;
    	pnode *current;
    	pnode *temp;
    	
    	pid_t pID;	
    	pID = 0;
    	
    	getnode(&list);
    	getnode(&current);
    	getnode(&temp);
    
    	list = NULL;
    	current = NULL;
    	temp = NULL;
    	
    	current = list;
    	
    	createprocesses(list, &current, &temp);
    	printf("PID1:  %d\n", list->pid);
    	list = list->next;
    	printf("PID2:  %d\n", list->pid);
    
    	exit(0);
    }
    
    void getnode(pnode **ptr)
    {
    	*ptr = malloc(sizeof(*ptr));
    	printf("Address of ptr: %p\n", ptr);
    }
    
    void insertnode(pnode **current, pnode **temp, int processid)
    {
    	if((*temp) != NULL){
    		getnode(&(*temp));
    	}
    	(*current)->pid = processid;
    	(*current)->next = (*temp);
    	(*current) = (*temp);
    }
    
    void startKilling(pnode *list)
    {	
    	printf("Merrily killing processes....\n");
    	printf("Anything after this should not exist.\n");
    }
    
    void createprocesses(pnode *list, pnode **current, pnode **temp) {
    	void insertnode(pnode **current, pnode **temp, int processID);
    	
    	pid_t pID;
    
    	pID = 0;
    	
    	while(pID != -1) {
    		pID = fork();
    		if(pID == 0) {
    			pause();
    		} else if(pID < 0) {
    			printf("Failed to fork...\n");
    		} else {
    			/* Parent Process */
    			printf("Process ID: %d\n", pID);
    			
    			insertnode(&(*current), (&(*temp)), pID);
    		}
    	}
    }
    Unfortunately, this STILL doesn't work for me. I think I may have mucked it up when either I called insertnode, or trying to figure out how the pointers should work inside of insertnode. I believe that I'm allocating AND setting the values of current, temp, and list to NULL outside of the function they're called in.

  11. #11
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    An expression like &(*x) is the same as x - just like 4 * x / 4 is the same as x. In the former case, the & does the opposite of *.

    So you can simplify your code by taking out all the &(*x) and leaving x on it's own.

    Second thing is that you don't need to pass around temp, do you? You always want to insert a new node, so insertnode would look like this:
    Code:
    void insertnode(pnode **current, int processid)
    {
            pnode *temp;
    	getnode(&temp);
    	(*current)->pid = processid;
    	(*current)->next = temp;
    	(*current) = temp;
    }
    You also need to set list to current at some point (or just remove all references to current, and use list instead) - all your inserting is done at the begining of the list, so you only need one single pointer - list is what you would want to call that in such a simple program.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

Popular pages Recent additions subscribe to a feed