1

Ok so I've only been doing C a few weeks and I have some problems. I have a priority queue filled with Bsig pointers. As an added feature, I want that whenever a Bsig is popped, we execute a custom function.

typedef struct bsig{
// some more fields here, but they're not important
    struct onpop_t *on_pop;
} Bsig;

typedef struct onpop_t {
    struct onpop_t *next;
    int (*do_on_pop)(Bsig *caller);
} Onpop;

int pop(SigPQ *q, Bsig* ret_sig){
    if (q->size < 1){
        return 0;
    }
    Bsig* signal = q->signals[0];
    assert(signal);
    q->signals[0] = q->signals[q->size-1];
    q->size--;
    sigpq_heapify(q,0);

    if(signal->on_pop){
        signal->on_pop->do_on_pop(signal);
    }
    ret_sig = signal;
    return 1;
}

So essentially, whenever we call pop, the do_on_pop function should be launched. Furthermore, pop takes a signal pointer which is overwritten by whatever is popped from the queue. All this happens in two files included by main.c. The following is from main (where testpop is a custom function that juts prints something random - it is declared and defined in the main.c file):

Bsig *sig1 = new_signal(1000, 0);
Onpop *onpop1 = malloc(sizeof(Onpop));
onpop1->do_on_pop = &testpop;
sig1->on_pop = onpop1;
push(pq, sig1);

Bsig *ret_sig;
pop(pq,ret_sig);

So far so good - the custom function testpop gets called. But

ret_sig->on_pop->do_on_pop(ret_sig);

from main gives a segmentation fault! I don't understand why. The ret_sig address and the signal address should be the same, and the function call is the same - the only difference is that one is called from main and one is called from the included .c file. Can anyone shed a light?

embedded_crysis
  • 193
  • 2
  • 12
  • 1
    As always with debugging C programs, run the program in a debugger and see where the segfault happens. Also inspect some of the values; are you dereferencing null somewhere? – Colonel Thirty Two Nov 16 '16 at 16:02
  • 1
    "main gives a segmentation fault" is simply not enough information for us to answer your question. Either you specify exactly which line of code gives the segmentation fault, or - preferably - do some debugging, investigate the problem a little deeper, and - if still needed - come back with a more accurate description of the problem that you're experiencing. – barak manos Nov 16 '16 at 16:04
  • will return with more info, if I don't solve it. But yes I found out that ret_sig == NULL. Thanks, robot commenter :) :) – embedded_crysis Nov 16 '16 at 16:06
  • Can youn post a [MCVE] ? – Jabberwocky Nov 16 '16 at 16:06
  • I suspect `sigpq_heapify(q,0);` fails when `q->size == 0`. Still, not enough info – chux - Reinstate Monica Nov 16 '16 at 16:06
  • 1
    There's too little information for anyone to be able to re-produce or diagnose the problem. However, my gut tells me it is most likely [this bug](http://stackoverflow.com/questions/39486797/dynamic-memory-access-only-works-inside-function) again. – Lundin Nov 16 '16 at 16:07
  • 2
    C parameters are passed by value. When you call `pop` with `ret_sig`, you are passing a copy of the pointer. When `pop` modifies `ret_sig`, it is modifying the local copy, which does not affect the copy that the caller has. You need to pass a pointer to the pointer (or change the function to return the pointer instead of the `int`). – pat Nov 16 '16 at 16:07
  • @pat Ah that sounds perfect. – embedded_crysis Nov 16 '16 at 16:10
  • You check that the pointer to structure is not null, but you don't check actually if the pointer to the functions is not null.... This can be the problem, but you only provide a code snippet, and nothing that can be compiled and tested locally, see comment by @embedded_crysis, and write a complete example to illustrate how you receive the `SIGSEGV`. – Luis Colorado Nov 17 '16 at 15:42

1 Answers1

2

You need to explicitly pass ret_sig by reference.

int pop(SigPQ *q, Bsig** ret_sig){
    if (q->size < 1){
        return 0;
    }
    Bsig* signal = q->signals[0];
    assert(signal);
    q->signals[0] = q->signals[q->size-1];
    q->size--;
    sigpq_heapify(q,0);

    if(signal->on_pop){
        signal->on_pop->do_on_pop(signal);
    }
    *ret_sig = signal;
    return 1;
}

And pass the address of ret_sig

Bsig *ret_sig;
pop(pq,&ret_sig);

Or change the function to return the pointer instead of an int (which is cleaner):

Bsig *pop(SigPQ *q){
    if (q->size < 1){
        return NULL;
    }
    Bsig* signal = q->signals[0];
    assert(signal);
    q->signals[0] = q->signals[q->size-1];
    q->size--;
    sigpq_heapify(q,0);

    if(signal->on_pop){
        signal->on_pop->do_on_pop(signal);
    }
    return signal;
}

And assign the return value:

Bsig *ret_sig = pop(pq);
pat
  • 12,587
  • 1
  • 23
  • 52
  • Nit picking: At least on language level there is no "*pass ... by reference*" in C. – alk Nov 16 '16 at 16:44
  • @alk. True, C always passes parameters by value. In this case, `&ret_val` is passed by value, which effectively (explicitly) passes `ret_val` by reference. – pat Nov 16 '16 at 16:47
  • Unless you count arrays, which are passed by reference because they decay into pointers automagically. – pat Nov 16 '16 at 17:32