HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Interprocess Communication in a Farmer-Worker setup

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
communicationworkersetupfarmerinterprocess

Problem

The following code is my first C program I have made for an university assignment about Interprocess Communication. This involved creating a parent process, the farmer, and then creating a certain number of forked processes, the workers, and we had to let the workers calculate something and use message queues to communicate it from the parent to the worker and vica versa. The computations were chosen to be Mandelbrot computations by the teacher. The goal of the assignment was to get familiar with the POSIX libraries.

These were the functional requirements, in my own words:

  • The program must use the settings from settings.h



  • The farmer starts NROF_WORKER worker processes



  • The message queues have a size of MQ_MAX_MESSAGES messages



  • The names of the message queues had to include the student name and the student id



  • The worker queue needs to be as full as possible



  • The farmer queue needs to be as empty as possible



  • Busy waiting is not allowed



  • We have to use a rsleep(10000) call to simulate some kind of random waiting, implementation given.



  • Lastly, another implicit requirement is that we cannot use extra processes or threads, we are only allowed to use the 1 farmer process and the NROF_WORKER worker processes.



The following implementations were given:

  • The code that outputs the image on the screen



  • The Mandelbrot computation



The Makefile that was provided:

#
#

BINARIES = worker farmer

CC = gcc
CFLAGS = -Wall -g -c
LDLIBS = -lrt -lX11

all:    $(BINARIES)

clean:
    rm -f *.o $(BINARIES)

worker: worker.o 

farmer: farmer.o output.o

worker.o: worker.c settings.h common.h

output.o: output.c output.h settings.h

farmer.o: farmer.c output.h settings.h common.h


This is the relevant code:

settings.h

```
#ifndef _SETTINGS_H_
#define _SETTINGS_H_

// remove the comments for the output you like: either graphical (X11) output
// or storage in a BMP file (or both)
#define WITH_X11
//#define WITH_BMP

// settings for interprocess

Solution

Generally speaking, the code looks good. It seems to be clean and to achieve what it is meant to achieve. Since your Makefile was given, as well as the compiler, I will review your code in function of what is legal with the -std=gnu89 compiler option, which means POSIX C89 + parts of C99 + some other compiler extensions. Here is a list of the allowed extensions.

-
First of all, your header guard names (_COMMON_H_, _OUTPUT_H_) start with an underscore followed by a capital letter, and is therefore a name reserved to the implementation (compiler + standard library). The simpler fix is to remove the leading underscore: COMMON_H_, INPUT_H_.

-
You use perror to display an error message when some computation fails. Do you want to keep it as is, or do you also want to make you program abort on errors? Currently, I don't think that resuming the execution after an error will lead to expected behaviour. While it is not that important if closing a resource fails, there ought to be some problems if opening one fails.

-
complex_dist is a bad name since it actually returns the square of the distance. You should change it to complex_square_dist or something equivalent. Note that the "distance" of a complex number is its absolute value that can be computed with cabs. Since you only need the square distance, I would simply rename your function.

-
if (first_call == true) is not the best way to check for a boolean value. You should write if (first_call) instead.

-
If there is no other possible outcome, returning EXIT_SUCCESS is useless, you could simply return 0 instead. Generally speaking, it is good to return EXIT_SUCCESS when you know that your program can also return error codes. Removing it may serve as kind of documentation to say "this program never returns error codes".

-
You are retrieving information from argv without checking first if they exist. You should use conditions such as `if (argc

Context

StackExchange Code Review Q#63713, answer score: 9

Revisions (0)

No revisions yet.