Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have this question in my exam and I want to know if my answer is correct.

Consider the following program written in C language to implement a reader:

// to simplify the exercise, are not checked the return values ​​of calls to the operating system

 int main(int argc, char* argv[]) {
     sem_t *sem_wrt;
     int fd, readcount;
     fd=shm_open("/readcount",O_RDWR|O_CREAT,S_IRUSR|S_IWUSR);
     ftruncate(fd, sizeof(int));
     readcount=mmap(0,sizeof(int),PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);
     sem_wrt=sem_open("/write",O_CREAT ,0xFFFFFFFF,1);
     while(1) {
        readcount++;
        if (readcount==1) sem_wait (wrt);
        my_read();
        (readcount)--;
        if (readcount==0) sem_post (wrt);
     }
 }

After some time of using this program, it was found that the program only works properly when it is executed only by a process simultaneously.

Change the code of the program in order to provide a solution to this problem.

int main(int argc, char* argv[]) {
     sem_t *sem_wrt;
     int fd, readcount;
     fd=shm_open("/readcount",O_RDWR|O_CREAT,S_IRUSR|S_IWUSR);
     ftruncate(fd, sizeof(int));
     readcount=mmap(0,sizeof(int),PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);
     sem_wrt=sem_open("/write",O_CREAT ,0xFFFFFFFF,1);
     //sem_mutex=sem_open("/mutex",O_CREAT ,0xFFFFFFFF,1);
     while(1) {
        //sem_wait(mutex);
        readcount++;
        if (readcount==1) sem_wait (wrt);
        //sem_post(mutex);
        my_read();
        //sem_wait(mutex);
        (readcount)--;
        if (readcount==0) sem_post (wrt);
        sem_post(mutex);
     }
 }
share|improve this question
add comment

1 Answer

int fd, readcount;
// ...
readcount=mmap(0,sizeof(int),PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);

If this is the code you're using, it probably gave you a warning which you ignored. mmap returns void *. You should change the declaration of readcount to int *. Also, the size of the mapping should be a multiple of the page size. I'd be sure to re-read the manual for mmap, carefully read any compiler warnings, and make sure your code checks for errors.

At any rate, if int *readcount; refers to shared memory, the following will not work:

++*readcount;

The problem is this line is conceptually broken into a few steps:

  1. Fetch *readcount into register
  2. Increment register
  3. Store back into *readcount.

If multiple processes perform steps 1/2/3 in lockstep, and the initial value is 0, they will both store 1 back.

You could use a GCC extension for compare and swap to make an increment safe:

int fetch;
do
{
   fetch = *readcount;
} while (!__sync_bool_compare_and_swap(readcount, fetch, fetch+1));

However I'm not sure if this will work right if you do it on memory mapped from a file by mmap. Keeping this kind of atomicity in a page fault handler seems complex and I'm not sure if the standard guarantees that it should work.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.