user2166141
user2166141

Reputation: 25

Segmentation fault in queue implementation with C

I have implemented a simple queue system in C. But I'm having problems with the function append. This doesn't happen every time, just a few times, but I cannot find the common denominator.

gdb says the segmentation fault is caused by the line while (*h){ but I think it is OK.

Here are the functions:

int pop (int *h){
    int ret = *h, i;

    for (i = 0; i < 52; i++){
        if (!*(h+i)){
            *(h+i-1) = 0;
            break;
        }
        else{
            *(h+i-1) = *(h+i);
        }
    }   
    return ret;
}


void append(int *h, int i){
    while (*h){
        ++h;
    }   
    *h = i;
}

Thank you very much.

A note: the queue size is fixed, and so the number of values that go into and out of it, so the problem isn't about going out of bounds.

EDIT

I have fixed it. Here are the functions working:

int pop (int *h){
    int ret = *h, i;

    for (i = 1; i < 52; i++){
        if (!h[i]){
            h[i-1] = 0;
            break;
        }
        else{
            h[i-1] = h[i];
        }
    }   
    return ret;
}


void append(int *h, int i){
    int j;

    for (j = 0; j<52; j++){
        if (!h[j]) break;
    }   
    h[j] = i;
}

Upvotes: 0

Views: 136

Answers (1)

Patrick Schl&#252;ter
Patrick Schl&#252;ter

Reputation: 11841

For gods sake, use the array notation [] instead of the pointer dereferencing *(). Here your code with the right notation and it gets obvious where the problem is.

int pop (int *h){
  int ret = *h, i;

  for (i = 0; i < 52; i++){    <--- Change to i=1
    if (!h[i]){                                                     
        h[i-1] = 0;        <------ Buffer underflow when h[0] == 0  
        break;                                                      
    }
    else{
        h[i-1] = h[i];     <------ Buffer underflow when h[0] != 0
    }
  }   
  return ret;
}   


void append(int *h, int i){   Where's the buffer overflow check ????
  while (*h){
    ++h;
  }   
  *h = i;
}

Have you also initialized your array with 0 values? Furthermore, is it really wanted that your stack/queue can not contain a 0 value?

EDIT: Here the corrected version

int pop (int *h)
{
  int ret = h[0], i = 1;
  do {
    h[i-1] = h[i];
  } while(h[i] && i<52);
  return ret;
}   


void append(int *h, int value)
{
int i;
  for(i=0; i<52; i++) {
    if(!h[i])
      break;
  }
  if(i<52)
    h[i] = value;
  else
    fprintf(stderr, "Array is full\n");
}

Upvotes: 1

Related Questions