Memory mapped, forks and unnamed semaphores- doesn't always give the same output

[neverMind]

Hi there,

I have an assignment to invert a ppm image, with a header of 4 lines (P6\nwidth\nheight\ndepth\n),and we have to do a shared memory version and a memory mapped file (MMF) too.
This version is basically equal to the SM version, whick worker, but with mmf, but the final image although is inverted it shows a small black line and a grey two, in the end, even the file has the same size ...
The more stranger is that yesterdar the output was correct and now I run and it's not. I think it's a problem with children not actually exit from their function, but I've tried so many changes and no success.

I'll leave you the code instead of the links, perhaps it's easier to see:

//###################################### MMF_INVERTER.C (main) #########################################
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/ipc.h>
#include <sys/shm.h>
#include <sys/fcntl.h>
#include <semaphore.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <errno.h>
#include <signal.h>
 
#include "timeprofiler.h"
#include "ppmtools.h"
 
#define    FILE_MODE    (S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
 
//Global vars:
int shmids[2];
//################# On shared memory goes #####################
//To share unnamed semaphores between processes, they must be allocated in a shared memory.
mem_struct *sh_mm;
int *total_lines, *processed_lines, *next_line, *buf_vars;
//###################################################################
//unnamed semaphores
sem_t *mutex1, *mutex2, *mutex3, *sem_remaining_lines;
pid_t *workersPID;
header *h;
char *data; //mmf that will hold the image
int last_index;
 
int main(int argc, char *argv[]) {
    int i, fdout,id;
    pixel *row;
    double start, stop, startms, stopms;
    struct stat sbuf;
 
    if (argc < 3) {
        printf("Incorrect usage.\nPlease use \"./invert input_filename.ppm output_filename.ppm\"\n");
        return -1;
    }
 
 
    printf("Opening input file [%s]\n", argv[1]);
    FILE *fpin = fopen(argv[1], "r");
    if (fpin == NULL) {
        printf("Could not open input file\n");
        return -1;
    }
 
    printf("Opening output file [%s]\n", argv[2]);
    FILE *fpout = fopen(argv[2], "w");
    if (fpout == NULL) {
        printf("Could not open output file\n");
        return -1;
    }
 
    printf("Getting header\n");
    h = getImageHeader(fpin);
    if (h == NULL) {
        printf("Error getting header from file\n");
        return -1;
    }
    printf("Got file Header: %s - %u x %u - %u\n", h->type, h->width, h->height, h->depth);
 
    init();
 
    //start timer
    start = getCurrentTimeMicro();
    startms = getCurrentTimeMili();
    printf("Saving header to output file\n");
    if (writeImageHeader(h, fpout) == -1) {
        printf("Could not write to output file\n");
        return -1;
    }
 
    last_index = (int)ftell(fpout);
    //printf("offset after header= %d\n",last_index);
 
    //alloc mem space for one row (width * size of one pixel struct)
    row = malloc(h->width * sizeof (pixel));
 
    /*Create a copy of the original image to the output file, which will be inverted*/
    printf("Starting work\n");
    for (i = 0; i < h->height; i++) {
        printf("Reading row... ");
        if (getImageRow(h->width, row, fpin) == -1) {
            printf("Error while reading row\n");
        }
        printf("Got row %d || ", (i + 1));
 
        printf("Saving row... ");
        if (writeRow(h->width, row, fpout) == -1) {
            printf("Error while reading row\n");
        }
        printf("Done\n");
    }
 
    /*Open file descriptor of the ouput file.
     * O_RDWR -  Read and Write operations both permitted
     * O_CREAT - Create file if it doesn't already exist
     * O_TRUNC - Delete existing contents of file*/
    if ((fdout = open(argv[2], O_RDWR, FILE_MODE)) < 0) {
        fprintf(stderr, "Can't create %s for writing\n", argv[2]);
        exit(1);
    }
 
    /*Get size of the output file*/
    if (fstat(fdout, &sbuf) == -1) {
        perror("Stat error ---------->\n");
        exit(1);
    }
    //printf("Size of output file: %d\n",(int)sbuf.st_size);
 
    /*Maps output file to memory*/
    if ((data = mmap((caddr_t) 0, sbuf.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fdout, 0)) == (caddr_t) (-1)) {
        perror("Error mmaping");
        exit(EXIT_FAILURE);
    }
    //printf("header last index: %u\n",last_index);
 
    sem_wait(mutex1);
    *total_lines = h->height;
    *next_line = last_index;
    sem_post(mutex1);
 
    //Creates workers
    workersPID = (pid_t*) malloc(sizeof (pid_t) *((NUM_WORKERS)));
    for (i = 0; i < NUM_WORKERS; i++) {
        id = fork();
        if (id == -1) {
            printf("Error creating worker number %d\n", i);
            exit(EXIT_FAILURE);
        } else if (id == 0) {
            workersPID[i] = getpid();
            worker(i);
        }
    }
    sem_wait(mutex2); //if unblocks, it means the inversion is finished
 
    printf("Cleaning up...\n");
    //clean up row
    free(row);
    //clean up header
    free(h);
 
    printf("Closing file pointers.\n");
    fclose(fpin);
    fclose(fpout);
    //stop timer
    stop = getCurrentTimeMicro();
    stopms = getCurrentTimeMili();
 
    for (i = 0; i < NUM_WORKERS; i++) {
        if (workersPID[i]) {
            kill(workersPID[i], SIGTERM); //TODO: sigkill or sigterm?
            waitpid(workersPID[i], NULL, 0);
        }
    }
 
    //unmmaps output file from memory
    if((munmap(data, sbuf.st_size)==-1))
    printf("munmap failed! Errno returned: %s\n",strerror(errno));
    close(fdout);
    terminate();
 
    printTimeElapsed(start, stop, "microseconds");
    printTimeElapsed(startms, stopms, "miliseconds");
    printf("Done!\n");
 
    return 0;
}
 
void init() {
 
    //unnamed semaphores go, in this case, to shared memory (is it REALLY necessary or as globlal vars it'll work?)
    if ((shmids[0] = shmget(IPC_PRIVATE, sizeof (mem_struct), IPC_CREAT | 0700)) == -1) {
        printf("shmget to allocate mem_Struct for semaphores failed. Errno returned %s\n", strerror(errno));
        exit(EXIT_FAILURE);
    }
    sh_mm = (mem_struct*) shmat(shmids[0], NULL, 0);
    if (sem_init(&sh_mm->mutex1, 1, 1) == -1) {
        printf("Error initializing semaphore mutex1.Errno returned: %s\n", strerror(errno));
        exit(EXIT_FAILURE);
    }
    mutex1 = &sh_mm->mutex1;
 
    if (sem_init(&sh_mm->mutex2, 1, 0) == -1) {
        printf("Error initializing semaphore mutex2.Errno returned: %s\n", strerror(errno));
        exit(EXIT_FAILURE);
    }
    mutex2 = &sh_mm->mutex2;
 
    if (sem_init(&sh_mm->mutex3, 1, 1) == -1) {
        printf("Error initializing semaphore mutex3.Errno returned: %s\n", strerror(errno));
        exit(EXIT_FAILURE);
    }
    mutex3 = &sh_mm->mutex3;
 
    if (sem_init(&sh_mm->sem_remaining_lines, 1, h->height) == -1) {
        printf("Error initializing semaphore sem_remaining_lines.Errno returned: %s\n", strerror(errno));
        exit(EXIT_FAILURE);
    }
    sem_remaining_lines = &sh_mm->sem_remaining_lines;
 
 
    /*Shared Memory segment for 3 integers -  MUST be like this*/
    if ((shmids[1] = shmget(IPC_PRIVATE, 3 * sizeof (int), IPC_CREAT | 0700)) == -1) {
        printf("shmget to allocate the 3 integers failed. Errno returned;  %s\n", strerror(errno));
        exit(EXIT_FAILURE);
    }
    buf_vars = (int*) shmat(shmids[1], NULL, 0);
    total_lines = &buf_vars[0];
    processed_lines = &buf_vars[1];
    next_line = &buf_vars[2];
 
 
}
 
/*Worker process*/
void worker(int i) {
    int cur_index = 0;
    //pixel *row;
    //Block all signals, except SIGINT and SIGKILL which are handled
    sigset_t block_ctrlc;
    sigfillset(&block_ctrlc);
    sigdelset(&block_ctrlc, SIGINT);
    sigdelset(&block_ctrlc, SIGTERM);
    sigprocmask(SIG_BLOCK, &block_ctrlc, NULL);
    signal(SIGINT, handle_signal);
    signal(SIGTERM, handle_signal);
 
    while (sem_wait(sem_remaining_lines) != -1) { //if there are still lines to read, go on
        sem_wait(mutex3);
        cur_index = *next_line; //current image's line
        *next_line += 3 * h->width; //refreshs line for the next worker
        sem_post(mutex3);
        printf("WORKER %d Inverting row... \n",i);
        invertRowOnMMF(3*h->width,data,cur_index);
        printf("Done || \n");
        sem_wait(mutex1);
        *processed_lines += 1; //increases the number of inverted lines
        if (*processed_lines == *total_lines) { //check if it reaches last line
            sem_post(mutex2); //if so, wakes the master telling that is ready
        }
        sem_post(mutex1);
    }
}
 
void handle_signal(int signum) {
    if (signum == SIGINT)
        signal(SIGINT, handle_signal);
    else
        signal(SIGTERM, handle_signal);
    exit(0);
}
 
void terminate() {
    int i;
    /*An unnamed semaphore should be destroyed with sem_destroy()
    before  the memory  in  which it is located is deallocated*/
    sem_destroy(mutex1);
    sem_destroy(mutex2);
    sem_destroy(mutex3);
    sem_destroy(sem_remaining_lines);
 
    //cleans up shared memory = removes shared memory segments
    for (i = 0; i < 2 ;i++) {
        shmctl(shmids[i], IPC_RMID, NULL);
    }
 
}
 
 

and now the other .c e .h needed:

#include <sys/types.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <sys/wait.h>
#include <math.h>
 
#include "ppmtools.h"
 
#define READ_BUFFER_LEN 16
 
char* readType(FILE *fp) {
    char buf[READ_BUFFER_LEN];
    char *t;
 
    //get line of the file into read buffer -> type of ppm
    t = fgets(buf, READ_BUFFER_LEN, fp);
    //check if it reads something and if it is a P6 type
    if (t == NULL) {
        printf("could not read type from file\n");
        return "";
    }
 
    if (strncmp(buf, "P6\n", 3) != 0) {
        printf("type is not P6\n");
        return "";
    } else {
        printf("set type as P6\n");
        //set type as P6
        return "P6";
    }
}
 
unsigned int readNextHeaderValue(FILE *fp) {
    char buf[READ_BUFFER_LEN];
    char *t;
    int r;
    int value;
 
    printf("read next header value\n");
    //load next line
    t = fgets(buf, READ_BUFFER_LEN, fp);
    if (t == NULL) {
        printf("Error reading from file");
        return 0;
    }
 
    //read value as unsigned int
    r = sscanf(buf, "%u", &value);
 
    //check if the read was successful
    if (r != 1) {
        printf("Could not read width value\n");
        return 0;
    } else {
        return value;
    }
}
 
header* getImageHeader(FILE *fp) {
    //alloc header
    header* h = malloc(sizeof (header));
 
    //check if file pointer is null
    if (fp == NULL) {
        //file pointer is null
        return NULL;
    }
 
    //get type
    char* type = readType(fp);
    if (strcmp(type, "") != 0) {
        //write type
        strcpy(h->type, type);
    } else {
        //error reading type
        return NULL;
    }
 
    h->width = readNextHeaderValue(fp);
    h->height = readNextHeaderValue(fp);
    h->depth = readNextHeaderValue(fp);
 
    //check for successful read and valid color range
    if (h->depth != 255 || h->width == 0 || h->height == 0) {
        //printf("error reading values\n");
        return NULL;
    }
 
    return h;
}
//NOTE:
// The program fails if the first byte of the image is equal to 32 (ascii for space char). because
// the fscanf in eats the space (ascii 32) and the image is read with one byte less
 
int getImageRow(int pixelNo, pixel* row, FILE *fp) {
 
    int i;
    byte b;
    //reading line
 
    for (i = 0; i < pixelNo; i++) {
        //set pixel values
        if (fread(&row[i], sizeof (b), 3 * sizeof (b), fp) < 1)
            return -1;
    }
 
    return pixelNo;
}
 
void invertRow(int pixelNo, pixel* row) {
    pixel aux;
    int i;
 
    //for each pixel
    for (i = 0; i < pixelNo / 2; i++) {
        //exchange pixel [i] with the pixel at the end minus [i]
        aux = row[i];
        row[i] = row[pixelNo - i - 1];
        row[pixelNo - i - 1] = aux;
    }
}
 
void invertRowOnMMF(int width, char *data, int offset) {
    char bytes1[3];
    char bytes2[3];
    int i, j, end_offset;
 
    //for each pixel
    for (i = offset, j = 0; j < width / 2; i += 3, j += 3) {
        end_offset= offset+width;
        //exchange pixel [i] with the pixel at the end minus [i]
        bytes1[0] = data[i]; //red
        bytes1[1] = data[i + 1]; //green
        bytes1[2] = data[i + 2]; //blue
 
        bytes2[0] = data[end_offset - j- 3]; //red
        bytes2[1] = data[end_offset - j - 2]; //green
        bytes2[2] = data[end_offset - j - 1]; //blue
 
        data[i] =     bytes2[0]; //red
        data[i + 1] = bytes2[1]; //green
        data[i + 2] = bytes2[2]; //blue
 
        data[end_offset - j - 3] = bytes1[0]; //red
        data[end_offset - j - 2] = bytes1[1]; //green
        data[end_offset - j - 1] = bytes1[2]; //blue
    }
}
 
int writeImageHeader(header* h, FILE* fp) {
    //write header fields with newline between them
    if (fprintf(fp, "%s\n%u\n%u\n%u\n", h->type, h->width, h->height, h->depth) < 1) {
        return -1;
    }
 
    return 0;
}
 
int writeRow(int pixelNo, pixel* row, FILE* fp) {
    int i, r;
 
    //for each pixel
    for (i = 0; i < pixelNo; i++) {
        //write one byte per RGB channel in pixel
        r = fwrite(&row[i], sizeof (byte), 3 * sizeof (byte), fp);
 
        //check for errors
        if (r < 1) {
            return -1;
        }
    }
 
    return 0;
}
 

the header file:

//###################################### PPMTOOLS.H #######################################
#include <semaphore.h>
 
#define NUM_WORKERS 2
 
 
//this is a convenient typedef to better read the code.
//it defines "byte" as a unsigned 8bit int.
typedef u_int8_t  byte;
 
//this struct represents each pixel.
//should be used to handle each pixel and maintain their correct colors.
typedef struct pixel_t {
    byte red;
    byte green;
    byte blue;
}pixel;
 
//this is the structure used do allocate the SH_MM to the image file
typedef struct {
    pixel *pixel_data;
} image_struct;
 
 
typedef struct{
    sem_t mutex1, mutex2, mutex3, sem_remaining_lines;
}mem_struct;
//this struct represents the ppm file header.
//it stores the file info and will be copied to the output file.
typedef struct header_t {
    char type[3];
    unsigned int width;
    unsigned int height;
    unsigned int depth;
}header;
 
//reads a line and extracts the type of ppm file.
//returns "P6" on success or "" on fail or other type of ppm file.
char* readType(FILE *fp);
 
//reads a line and extracts the unsigned int value of that line.
//returns 0 on fail and a value greater than 0 ond success.
//used to read width, height and depth from the ppm file.
unsigned int readNextHeaderValue(FILE *fp);
 
//reads file and extracts the header.
//returns a header struct on success or NULL on failure.
header* getImageHeader(FILE *fp);
 
//reads a complete row of the image based on the number of pixels given.
//user should give an initialized pixel array to be filled.
//returns number of pixels read on success and -1 on failure.
int getImageRow(int pixelNo, pixel* row, FILE *fp);
 
//inverts the pixels in the given row.
void invertRow(int pixelNo, pixel* row);
 
//writes the image header to file.
//returns 0 on success and -1 on failure.
int writeImageHeader(header* h, FILE* fp);
 
//writes a row of pixels to a file.
//returns 0 on success and -1 on failure
int writeRow(int pixelNo, pixel* row, FILE* fp);
 
//initiates sems and allocates shared memory
void init();
void worker(int i);
void handle_signal(int signum);
//cleans everything
void terminate();
void invertRowOnMMF(int width, char *data, int offset);
 

and the makefile:

FLAGS = -Wall -g -lrt
CC = gcc
EXEC2 = MMF_inverter
OBJS2 = ppmtools.o MMF_inverter.o
 
all: ${EXEC2} clean
 
clean:
    rm *o
 
${EXEC2}:    ${OBJS2}
    ${CC} ${FLAGS} ${OBJS2} -o $@    
 
.c.o:
    ${CC} ${FLAGS} $< -c
 
 
# # # # # # # # #
 
ppmtools.o: ppmtools.c
MMF_inverter.o: MMF_inverter.c