1
int main(int argc, char *argv[])
{
  printf("successfully started main\n");
  struct uf_list myList;
  uf_list_allocate(&myList);
  printf("successfully allocated myList\n");
  insert_node(&myList, 'c');
  printf("successfully inserted into myList\n");

  return 0;
}

...

void uf_list_allocate(struct uf_list *list)
{
  list = malloc(sizeof(struct uf_list));
  if(list == NULL)
    {fprintf(stderr, "no memory for allocate");}
  list->head = list->tail = NULL;
}
//--------------------------------------------------------------------------------------
void insert_node(struct uf_list *list, const char label)   
{
  struct uf_node *it = malloc(sizeof(struct uf_node));
  if(it == NULL)
    {fprintf(stderr, "no memory for insert");}

  it->c = label;
  it->next = NULL;                                     
  it->rep = NULL;

  if(list->head == NULL)                                //the list is empty
    { list->head = list->tail = it;}
  else
    { list->tail->next = it; list->tail = it; }

  it->rep = list->head;
}
/*----------------------------------------------------------------------------*/
struct uf_node
{
  char c;
  struct uf_node *next;
  struct uf_node *rep;
};
/*----------------------------------------------------------------------------*/
struct uf_list
{
  struct uf_node *head;
  struct uf_node *tail;
};

I am getting a segmentation fault once I try to insert an element into my list from main. What is causing the segmentation fault? If you need any more information such as the definitions of the structs let me know!

EDIT: I realize what I did. Inside allocate I changed the address of local variable list. This means that nothing has happened to myList. However, now I have the following puzzle: I placed the declaration of myList outside of main, and everything works:

struct uf_list myList;

int main(int argc, char *argv[])
{
  printf("successfully started main\n");
  uf_list_allocate(&myList);
  printf("successfully allocated myList\n");
  insert_node(&myList, 'c');
  insert_node(&myList, 'd');
  insert_node(&myList, 'e');
  printf("successfully inserted into myList\n");
  print_uf_list(&myList); 


  return 0;
} 

I can't quite figure out why. It appears that the same logic should apply, namely, since I pass the address of myList into allocate but then change the local variable list address and operate on that address, how is this being reflected on myList whose memory address is not being operated on?

9
  • 3
    -1: You have 1.2k rep, surely you know better than to ask a question about a seg-fault without doing some basic debugging first. Mar 31, 2014 at 19:57
  • 1
    I put a bunch of print statements everywhere with no luck... I rarely program in C, most everything I do is in higher level languages. Everything here seems logical to me. I don't know what else to do... Mar 31, 2014 at 19:59
  • step through and see what the last line that executes is maybe? and where's the declaration of the list struct? The problem may well be start from uf_list_allocate() which you didn't post.
    – RobP
    Mar 31, 2014 at 20:00
  • BTW if malloc fails, the code should not continue. Best oractice... Mar 31, 2014 at 20:02
  • It's a doubly linked list right? so it->rep should point to the previous node and not the list->head?
    – avmohan
    Mar 31, 2014 at 20:02

2 Answers 2

1

In allocate, you dont return anything. Tat is the problem. In main, you should have just a pointer as local variable, and assign to it what the allocator function returns.

EDIT

Even more simple, since it is already allocated ( on the stack of main ), you can just remove the allocating code from that function, and have an initializing function. This is all you need:

  Uf_list_init(struct uf_list *list)
 {
    list->head = list->tail = NULL;
 }

In the original code:

list = malloc(sizeof(struct uf_list));

You have a pointer to te struct, but you overwrite it with a brand new pointer.

13
  • Wait, I passed the address of myList, so why can't I change the values for head and tail? Mar 31, 2014 at 20:15
  • 1
    I dont recall exactly, but that might be part of the standard, you can google it. But variables on stack surely don't get automatically inited with any values, they just start their life filled with random bits. Mar 31, 2014 at 20:46
  • 1
    Yup, found it stackoverflow.com/questions/14049777/… Mar 31, 2014 at 20:48
  • 1
    Also note, NULL pointer is not required to be equal to zero, so relying on zero filled memory containing NULL pointers is not strictly portable, infect against best practice. So please don't learn C relying on it. Mar 31, 2014 at 20:51
  • 1
    Anyways, it is very bad practice to rely on it, allocate is not strictly superfluous, eliminating it this way is an ugly hack. Mar 31, 2014 at 21:09
0

C passes parameters by value. uf_list_allocate should take uf_list **listRef so that it can be modified.

#include <stdio.h>
#include <stdlib.h>

struct uf_node
{
  char c;
  struct uf_node *next;
  struct uf_node *rep;
};

struct uf_list
{
  struct uf_node *head;
  struct uf_node *tail;
};

void uf_list_allocate(struct uf_list **listRef)
{
  *listRef = malloc(sizeof(struct uf_list));
  if(*listRef == NULL)
    {fprintf(stderr, "no memory for allocate"); exit(0);}
  (*listRef)->head = (*listRef)->tail = NULL;
}

void insert_node(struct uf_list *list, const char label)
{
  struct uf_node *it = malloc(sizeof(struct uf_node));
  if(it == NULL)
    {fprintf(stderr, "no memory for insert"); exit(0);}

  it->c = label;
  it->next = NULL;
  it->rep = NULL;

  if(list->head == NULL)                                //the list is empty
    { list->head = list->tail = it;}
  else
    { list->tail->next = it; list->tail = it; }

  it->rep = list->head;
}

int is_empty(const struct uf_list *list)
{
  return list->head == NULL;
}

void remove_node(struct uf_list *list)
{
  if (is_empty(list))
  {
    printf("List underflow\n"); exit(0);
  }
  else
  {
    struct uf_node *oldhead = list->head;
    list->head = list->head->next;
    if (list->tail == oldhead)
      list->tail = NULL;
    free(oldhead);
    printf("Node removed\n");
  }
}

void deallocate(struct uf_list **listRef)
{
  struct uf_list *list = *listRef;
  if(!is_empty(list))
  {
    while(!is_empty(list))
      remove_node(list);
  }
  free(list);
  list = NULL;
  printf("List deallocated\n");
}

void printList(const struct uf_list *myList)
{
  struct uf_node *cur = myList->head;
  while(cur!=NULL)
  {
    printf("%c -> ", cur->c);
    cur = cur->next;
  }
  printf("\n");
}

int main(int argc, char *argv[])
{
  printf("successfully started main\n");
  struct uf_list *myList;
  uf_list_allocate(&myList);
  printf("successfully allocated myList\n");

  insert_node(myList, 'c');
  printf("successfully inserted c into myList\n");

  insert_node(myList, 'd');
  printf("successfully inserted d into myList\n");
  printList(myList);

  insert_node(myList, 'e');
  printf("successfully inserted e into myList\n");
  printList(myList);

  remove_node(myList);
  printf("successfully removed c (head) from myList\n");
  printList(myList);

  deallocate(&myList);

  return 0;
}
4
  • 1
    It is a stack variable of main, can't modify its address Mar 31, 2014 at 20:11
  • Of course, you should make appropriate changes in main also.
    – avmohan
    Mar 31, 2014 at 20:15
  • I just placed the declaration of myList outside of main, and it works. Why? Global variables are allocated in the data segment correct? Mar 31, 2014 at 20:17
  • You can't modify myList in a function unless it's a global variable or it's passed by reference. I've addded a bit of code to remove_node (removes head node), deallocate (empties & deallocates the whole list) and a few more examples. It makes it a bit clearer. Hope you get it now. I'm posting the whole code - insert_node is unchanged except that it aborts execution if malloc fails.
    – avmohan
    Mar 31, 2014 at 20:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.