Thread: Segmentation fault when executing bounded buffer program

This is my first post, and I'm fairly new at programming. I have a project where I'm recreating the well known bounded buffer problem (producer-consumer). Upon creating these two threads, they share a buffer of size 10. When I compile the code I get a warning in the function 'write_to_buffer' stating "comparison between pointer and integer". When I execute the program, I receive the following statement "Segmentation Fault".

Can somebody explain what this means and provide any hints on how to fix it?

Thank you very much.

Code:

/*Header files*/
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <thread.h>
#include <semaphore.h>
#include <stdbool.h>
#define TRUE 1
#define FALSE 0

/*Function declarations*/
void *consumer (void *arg);
void *producer (void *arg);
void *read_from_buffer (char []);
void *write_to_buffer (char []);

/*Global declarations*/
char strng[10];
sem_t mutex, full, empty;

int main()
{

  /*-----------------Variable declarations--------------------------*/
  /*----------------------------------------------------------------*/
  int cons, prod, n;
  pthread_t tid1[1];
  pthread_t tid2[1];
  pthread_attr_t attr[1];


  /*Required to schedule thread independently*/
  pthread_attr_init(&attr[0]);
  pthread_attr_setscope(&attr[0], PTHREAD_SCOPE_SYSTEM);

  /*Creating 2 independent threads, Producer and Consumer*/
  prod = pthread_create(&tid1[0], &attr[0], producer, NULL);
  cons = pthread_create(&tid2[0], &attr[0], consumer, NULL);

  /*Initializing semaphores*/
  sem_init(&mutex,0,1);
  sem_init(&full,0,0);
  sem_init(&empty,0,10);

  /*---------End of variable declarations---------------------------*/
  /*----------------------------------------------------------------*/

  /*Destroy attribute and wait for threads*/
  pthread_attr_destroy(&attr[0]);

  /*Wait for threads to finish before exiting*/
  pthread_join(tid1[0],NULL);
  pthread_join(tid2[0],NULL);

/*Destroy semaphores*/
sem_destroy(&mutex);
sem_destroy(&full);
sem_destroy(&empty);

exit(0);

}

/*-------------------------------------------------------------------*/
/*-------------------------------------------------------------------*/
void *consumer (void *arg)
{
  while (TRUE)
 { /*Entry section*/
    sem_wait(&full);
  /*End of entry section*/

  /*Critical section*/
  sem_wait(&mutex);
  read_from_buffer (strng);
  /*End of critical section*/

  /*Remainder section*/
  sem_post(&mutex);
  sem_post(&empty);
}
  exit(0);
}

/*-------------------------------------------------------------------*/
/*-------------------------------------------------------------------*/
void *producer (void *arg)
{
  while (TRUE)
 {   /*Entry section*/
    sem_wait(&empty);
  /*End of entry section*/

  /*Critical section*/
  sem_wait(&mutex);
  write_to_buffer(strng);
  /*End of critical section*/

  /*Remainder section*/
  sem_post(&mutex);
  sem_post(&full);
  /*End of remainder section*/
}
  exit(0);
}

/*------------------------------------------------------------------*/
/*------------------------------------------------------------------*/
void *write_to_buffer (char buffer[])
{
int i=0;
char newchar;
FILE *fp;

/*Open the input file*/
fp = fopen("/home/csee1/jbarnes3/in.dat", "r");

if ((fp = fopen("in.dat", "r")) == NULL)
  printf("File not found\n");

while(fgets(buffer, 1, fp) !='*')
{
  buffer[i] = newchar; /*store the character entered*/
  ++i;
}
 buffer[i] = '*'; /*terminate the string*/

/*Close the file*/
close(fp);
return;
}

/*-----------------------------------------------------------------*/
/*-----------------------------------------------------------------*/
void *read_from_buffer (char buffer[])
{
int j;

while (buffer[j] !='*')
{
  printf("%c\n");
  puts (buffer);
  sleep(1);
  ++j;
}
return;
}

Originally Posted by Megalodon01

Code:

/*------------------------------------------------------------------*/
/*------------------------------------------------------------------*/
void *write_to_buffer (char buffer[])
{
   int i=0;
   char newchar;
   FILE *fp;

   /*Open the input file*/
   fp = fopen("/home/csee1/jbarnes3/in.dat", "r");

   if ((fp = fopen("in.dat", "r")) == NULL)
      printf("File not found\n");

   while(fgets(buffer, 1, fp) !='*')
   {
      buffer[i] = newchar; /*store the character entered*/
      ++i;
   }
   buffer[i] = '*'; /*terminate the string*/

   /*Close the file*/
   close(fp);
   return;
}

In the future, please indent your code properly and consistently.

Why are you opening your input file twice? Furthermore, you check to see if the FILE* returned is NULL, and print an error message if it is... but then you let the program continue... Don't you see a problem with this?

Also, you aren't storing anything in newchar, but yet you copy newchar into your buffer.

I didn't look at the rest of the program.

Thanks for looking at that portion of code, but instead of receiving "Segmentation Error" upon executing I receive no output.

Here's the portion of code I modified

Code:

void *write_to_buffer (char buffer[])
{
  int i=0;
  char newchar;
  FILE *fp;

  /*Open the input file*/
  fp = fopen("/home/csee1/jbarnes3/in.dat", "r");

   if (fp == NULL)
   {
     printf("\nFailed to open the file.\n");
     exit(1);
   }

   while(fgets(buffer, 10, fp) !='*')
   {
     ++i;
   }
   buffer[i] = '*'; /*terminate the string*/

  /*Close the file*/
  close(fp);
  return;
}

Thanks for helping.

Code:

void *read_from_buffer (char buffer[])
{
   int j;

   while (buffer[j] !='*')
   {
      printf("%c\n"); // You are telling printf that you will pass it a char (%c)
                      // but you aren't passing it a char...
      puts (buffer);
      sleep(1);
      ++j;
   }
   return;
}

Code:

void *write_to_buffer (char buffer[])
{
   ...
   // You keep reading into the same buffer at the same spot!
   // Every time you fgets() into 'buffer', you're
   // overwriting what you put there last time!
   while(fgets(buffer, 10, fp) !='*') // fgets() returns the char* that you passed it (in this
                                      // case 'buffer' is returned.  Why are you comparing that to
                                      // a literal '*' character?  That doesn't make any sense.
   {
      ++i;
   }
   buffer[i] = '*'; /*terminate the string*/
   ...
}

A couple more things:

You don't need to terminate your strings with a '*'. Strings in C are NULL-terminated (meaning the last character in every string should be the byte value 0x00). All of the C standard library functions that work with strings are written to look for that NULL-terminator. If you include a string literal in your program (like "Hello world!"), the compiler adds the NULL terminator on the end for you.

If you read the documentation for the C standard library functions, they will tell you exactly how they work with NULL terminators. For example, this is from the fgets() documentation:

Code:

char * fgets ( char * str, int num, FILE * stream );
str
    Pointer to an array of chars where the string read is stored.
num
    Maximum number of characters to be read (including the final null-character). Usually, the length of the array passed as str is used.
stream
    Pointer to a FILE object that identifies the stream where characters are read from.
    To read from the standard input, stdin can be used for this parameter.

From that you can tell that if you call fgets(buffer, 10, fp);, it will read 9 characters from the file and then add the NULL terminator on the end as the 10th character.

You're probably going to need to know how much you read into the buffer every time you do a read. fgets() doesn't tell you that. It may be more straightforward in this case to read the input one character at a time (using fgetc()) so that you can keep track of how many characters you read.

I was using '*' as a NULL so when the producer read this value it would terminate.

I modified my function to this

Code:

void *write_to_buffer (char buffer[])
{
  int i=0;
  FILE *fp;

  /*Open the input file*/
  fp = fopen("/home/in.dat", "r");

  if (fp == NULL)
    {
      printf("\nFailed to open the file.\n");
      exit(1);
    }

   else
     {
       printf("File is opened. Time to work.\n");

   while(TRUE)
   {
     newchar = fgetc(fp);

   if(newchar!= nul)
   {
     printf("%c",newchar);
   }

   else
   {
     break;
   }
   }

  /*Close the file*/
  fclose(fp);
  return 0;
   }
}

/*-----------------------------------------------------------------*/
/*-----------------------------------------------------------------*/
void *read_from_buffer (char buffer[])
{
  int j;

  while (buffer[j] !=nul)
  {
    printf("%c\n", &buffer);
    sleep(1);
    ++j;
  }
  return 0;
}

Now, I'm getting a weird output. The statement, "File is opened. Time to work." as well as the contents of the file I'm opening are printed 10 times. Is this because I'm trying to open the file in this function. Should it go in main instead?

Thanks for your help.

where is newchar declared?

I had 'new char' declared as a global variable, but seeing how I only use it in this function I'll declare it as local.

What exactly you expect to receive here?

I was expecting to obtain the characters located in the shared buffer. Upon executing my code, I receive nothing. Any hints?