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.

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

Quote:

---

Originally Posted by matsp

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

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?

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

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:

Quote:

---

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

---

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

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.

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 = %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

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.

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