0

I have written a helper function that was designed to add a node to a linked list. The location this was to be done with the following segment.

tcb_add(running,temp);

This segment is supposed to add temp to running. Here is my code for the function.

void tcb_add(tcb_t *first, tcb_t *second) { //fix
  if (first == NULL) {
    first = second;
  } else {
    while (first->next != NULL) {
      first = first->next;
    }
    first->next = second;
  }
}

If instead of using the line tcb_add(running,temp); I just say running = temp it works. But for other parts of my code, I would like to have the function since it was only that easy since temp was the first node being added. Any help would be very appreciated.

There is, of course, more code but I believe I isolated the problem.

Here is the output from Valgrind when I call tcb_add(running,temp);, but keep in mind when I instead say running = temp the segment works.

==30628== Memcheck, a memory error detector
==30628== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==30628== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==30628== Command: ./test00
==30628==
==30628== Invalid read of size 8
==30628==    at 0x400930: t_create (t_lib.c:42)
==30628==    by 0x4007F9: main (test00.c:25)
==30628==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==30628==
==30628==
==30628== Process terminating with default action of signal 11 (SIGSEGV)
==30628==  Access not within mapped region at address 0x8
==30628==    at 0x400930: t_create (t_lib.c:42)
==30628==    by 0x4007F9: main (test00.c:25)
==30628==  If you believe this happened as a result of a stack
==30628==  overflow in your program's main thread (unlikely but
==30628==  possible), you can try to increase the size of the
==30628==  main thread stack using the --main-stacksize= flag.
==30628==  The main thread stack size used in this run was 8388608.
==30628==
==30628== HEAP SUMMARY:
==30628==     in use at exit: 67,432 bytes in 4 blocks
==30628==   total heap usage: 4 allocs, 0 frees, 67,432 bytes allocated
==30628==
==30628== LEAK SUMMARY:
==30628==    definitely lost: 24 bytes in 1 blocks
==30628==    indirectly lost: 936 bytes in 1 blocks
==30628==      possibly lost: 0 bytes in 0 blocks
==30628==    still reachable: 66,472 bytes in 2 blocks
==30628==         suppressed: 0 bytes in 0 blocks
==30628== Rerun with --leak-check=full to see details of leaked memory
==30628==
==30628== For counts of detected and suppressed errors, rerun with: -v
==30628== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

Edit

The variable running is defined as follows:

tcb_t *running;
tcb_t *ready;

This is according to the struct:

struct tcb_t {
    int         thread_id;
    int         thread_priority;
    ucontext_t *thread_context;
    struct tcb_t *next;
}; typedef struct tcb_t tcb_t;

As asked this is my t_create function:

int t_create(void (*fct)(int), int id, int pri) {
  size_t sz = 0x10000;
  ucontext_t *uc;
  uc = (ucontext_t *)malloc(sizeof(ucontext_t));

  getcontext(uc);
  uc->uc_stack.ss_sp = malloc(sz); /* new statement */
  uc->uc_stack.ss_size = sz;
  uc->uc_stack.ss_flags = 0;
  uc->uc_link = running->thread_context;

  makecontext(uc, (void (*)(void))fct, 1, id);
  tcb_t *new_tcb = malloc(sizeof(tcb_t));
  new_tcb->thread_context = uc;
  new_tcb->thread_id = id;
  new_tcb->thread_priority = pri;
  new_tcb->next = NULL;
  tcb_t *tmp = ready;
  if (tmp == NULL) // I would like to replace this portion with tcb_add()
    ready = new_tcb;
  else {
    while (tmp->next != NULL) {
      tmp = tmp->next;
    }
    tmp->next = new_tcb;// To here
  }

Also to further explain although the valgrind report mentions t_create this is where I believed the problem took place.

void t_init() {
  tcb_t *temp = tcb_create();
    temp->thread_context = (ucontext_t *) malloc(sizeof(ucontext_t));
    temp->thread_id = 0;
    temp->thread_priority = 0;
    temp->next = NULL;

    getcontext(temp->thread_context);

    tcb_add(running,temp); //DOES NOT WORK
  //running = temp;        //WORKS
  ready = NULL;
}
Community
  • 1
  • 1
Anthony Sette
  • 777
  • 1
  • 10
  • 26
  • 1
    What is the value of `running ` before you call `tcb_add(running, temp)`? – Ray Toal May 18 '19 at 00:12
  • 2
    Your Valgrind log says you dereferenced a null pointer, and log points towards `t_create (t_lib.c:42)`. What is there? – milleniumbug May 18 '19 at 00:13
  • @RayToal running has no value it is just initialized. temp is given a value then assigned to running, or should be. Does that answer your question? Please look at the edits I made. – Anthony Sette May 18 '19 at 00:19
  • @milleniumbug This confused me too but the call of `tcb_add()` was not in `t_create()`, and when I replaced it with `running = temp` it worked. This was in `t_itit()`, would it help if I added that? Check the update. – Anthony Sette May 18 '19 at 00:21

1 Answers1

3

If instead of using the line tcb_add(running,temp); I just say running = temp it works

running is passed by value. Therefore the function has no way of modifying it. You have to pass a pointer to pointer.

void tcb_add(tcb_t **first, tcb_t *second) { //fix
  if (*first == NULL) {
    *first = second;
  } else {
    while ((*first)->next != NULL) {
      *first = (*first)->next;
    }
    (*first)->next = second;
  }
}

// ...

tcb_add(&running,temp);

To elaborate:

void f(int x) { x = 42; }

int x = 23;
f(x);
printf("%d\n", x); // WILL PRINT 23

in your case, the variable never changed, and kept the original NULL value.

milleniumbug
  • 15,379
  • 3
  • 47
  • 71