Segmentation fault in queue implementation with C

Question

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;
}

not "about going out of bounds." ? When i is 0, you are referencing *(h+i-1), which is h[-1] .. — xtof pernod
– xtof pernod, Commented Apr 22, 2013 at 20:00
So... you can never ever insert a value of 0 into your queue?! — Kerrek SB
– Kerrek SB, Commented Apr 22, 2013 at 20:02
How is the while (*h) loop expected to stop? Were elements 0-initialized? — Arun
– Arun, Commented Apr 22, 2013 at 20:02

Patrick Schlüter · Accepted Answer · 2013-04-22 20:18:28Z

1

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");
}

edited Apr 22, 2013 at 20:18

answered Apr 22, 2013 at 20:06

Patrick Schlüter

12k1 gold badge45 silver badges50 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

user2166141 Over a year ago

I have initialized my array with 0 values, and the queue cannot contain 0 values.

Collectives™ on Stack Overflow

Segmentation fault in queue implementation with C

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related