Errors with Peterson's Algorithm

The fork() and wait() functions return -1 when they fail. You do not test for that.

Moreover, all processes involved in your program call wait(). That call will certainly fail (returning -1) in at least one process, as regardless of the success or failure of the fork()'s, there will be one process without any child processes.

Your program has more serious problems, however. It appears to assume that ordinary global variables flag and turn will be shared between processes, but since I see you using shmget(), I presume you know that that won't actually happen.

And even if you move flag and turn into your shared memory segment, you still have a synchronization problem: you cannot rely on one process to see the other's modifications to shared memory in a timely manner -- or, in fact, ever -- without employing appropriate synchronization. The synchronization aid that naturally goes with SysV-style shared memory segments is the SysV-style semaphore.

And when you do implement synchronization, it should naturally fall out that you no longer need to use busy loops to make each process wait on the other.

Watch what you share

Your global declarations

int flag[2] = {FALSE, FALSE};
int turn = 0;

are not shared memory. Your total is shared memory because you share it, but the flag and turn are not. Each process has its own copy of the flag and turn and modify only their own copy of it. Since the synchronization mechanism you're trying to implement (Peterson's Algorithm) relies on both processes modifying the same flag and turn, it doesn't work. When one process changes the flag, the other doesn't know about it.

(The above has since been addressed in an edit to the question.)

Sharing and synchronizing between processes is typically more complicated than sharing and synchronizing between threads. The two are different things for different purposes. If it is possible, I'd consider using threads instead of processes. This way, sharing memory is trivial. This problem seems to be an educational exercise, and schools typically teach processes before threads.

Watch how you share it

You will need to initialize turn and flag after creating the shared memory, such as where you initialize total->value = 0; and not in a typedef declaration.

Go through your notes about how to set up shared memory and read through the relevant man pages.

In smget() you have a size of sizeof(int), which is not enough for the whole struct. Personally, I'm against hard coding types in sizeof() operators. I suggest using something like sizeof(*shared_mem) and let the compiler figure out what the type and size should be. It's also easier if you change the declaration for some reason, because you don't have to sift through your code looking for sizeof() operators.

Restructure the fork() calls and test the return values

Library functions return values for a reason. Ignoring what they say is like ignoring alarm bells. Sometimes such an alarm might help you figure out what is wrong with your program.

There are three return states to check from a fork() (check all of them):

• -1, an error occurred
• 0, in child process
• >0, in parent process (with child's PID)

Also, notice that after completing process1() and process2(), the children return to main() and fall through the subsequent instructions, even trying to wait for children they don't have. This is probably not what you want.

Check the return value from wait()

The -1 printed is the return value from wait(). This means that wait() encountered an error. The likely cause for this is that the calling process has no child to wait for.

From POSIX.1-2008/IEEE Std 1003.1-2008 (2013 Ed.) for the function wait():

pid_t wait(int *stat_loc);
pid_t waitpid(pid_t pid, int *stat_loc, int options);

. . .

If wait() or waitpid() return because the status of a child process is available, these functions shall return a value equal to the process ID of the child process. In this case, if the value of the argument stat_loc is not a null pointer, information shall be stored in the location pointed to by stat_loc.

I am interpreting this to mean that if wait() did not "return because the status of a child process is available", then we can not rely on the value written to stat_loc.

In other words, you should have checked the return value from wait() before using your status. A garbage value (status is uninitialized) could possibly evaluate WIFEXITED(status) to non-zero (true). If wait() returns an error, you shouldn't rely on the value of status. You can also use errno to check what went wrong (be sure to set errno = 0 before the call to wait()). See documentation for details.

Some comments about pointers

The lines

char *shmadd;
shmadd = (char *) 0;

are equivalent to char *shmadd = NULL;. You don't have to type cast it. (Do you even need the local variable shmadd? Can you just use NULL instead?)

Similarly, in

total = (shared_mem *) shmat (...)

you don't have to type cast the return value because it is a void *. (C++ or C# might make you do this, but not in C.)

Watch the variables

You have a second variable declaration of a variable named k that shadows the first one. Here is the declaration of the second one:

while(total->flag[1] == TRUE && total->turn == 1)
{
  int k = 0;
  if(k = 0)
  {
    total->p2Block = total->p2Block + 1;
  }
  k++;
};

This int k shadows the earlier k. Any reference to k in this block will use this one instead of the earlier one. There are several other problems with this code:

• if (k = 0) is an assignment of 0 to k, not a comparison. Since this evaluates to 0 (interpreted as false), the block will never be executed. The if has no effect.
• If you change this to if (k == 0) (a comparison) it still makes no sense since it will always be true. You just assigned 0 to k then check to see if k is 0. Why would it change?
• The k++ also has no logical meaning. This k was declared at the beginning of the block, and its scope is just this block. The next iteration of the while loop pushes a new k and initializes it to 0. So there is no logical effect of incrementing it.
• Why write total->p2Block = total->p2Block + 1 instead of just total->p2Block++?

This may explain why your counting is not behaving as intended.