beginners problem

Discussion in 'C Programming' started by Lars, Feb 11, 2008.

  1. Lars

    Lars Guest

    Hi all
    If this is the wrong list for beginners trouble I apologize.
    please refer to me the correct list in such case.

    I am trying to create a simple linked list but I keep getting segmentation
    errors and I dont know why. I realize that I do not free the allocated
    memory but the segfault happpens (it seems) with the insert function.
    Can any one explain ?

    Regards Lars

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

    typedef struct node {
      struct node * next;
      int data;
    } Node;


    Node * alloc_node(int n);

    void insert(Node * head, int n);

    int main(){
      Node * nod = NULL;
      insert(nod,4);
      printf("%d\n",nod->data);
      printf("%10p\n", nod);
      printf("%10p\n", nod->next);
      return 0;
    }

    Node * alloc_node(int n){
      Node * tmp;
      tmp = (Node *) malloc(sizeof(Node));
      tmp->data = n;
      tmp->next = NULL;
      return tmp;
    }

    void insert(Node * head, int n) {
      Node * tmp;
      if (head==NULL) {
        head = alloc_node(n);
      }
      else {  
        tmp = alloc_node(n);
        head->next = tmp;
      }
    }
     
    Lars, Feb 11, 2008
    #1
    1. Advertisements

  2. Lars

    Ian Collins Guest

    There's your problem, nod is NULL.
    You don't have to (and shouldn't) cast the return value of malloc.

    tmp = malloc(sizeof *tmp);
     
    Ian Collins, Feb 11, 2008
    #2
    1. Advertisements

  3. Lars

    Lars Guest

    Thanks for your reply. But is this not handled by the the insert functio
    through this ?
    if (head==NULL) {
        head = alloc_node(n);
      }

    Regards
    Lars
     
    Lars, Feb 11, 2008
    #3
  4. Lars

    Ian Collins Guest

    No, ant changes to head are local to the function, heat is a pointer, if
    you wanted the change to be visible to the caller, it would have to be a
    pointer to a pointer.

    void insert(Node** head, int n);
     
    Ian Collins, Feb 11, 2008
    #4
  5. Lars

    Lars Guest

    Thats it!!!
    I completely overlooked that.
    Thank you very much.
    Best regards
    Lars
     
    Lars, Feb 11, 2008
    #5
  6. Lars

    Richard Bos Guest

    This is not a list, it's a newsgroup. And if you post questions about
    ISO C, not about, oh, I dunno, M$VC#++4.6 or Ganuck 4.77.35.88.55-alpha,
    you should be fine here whether you're a beginner or not. Your question
    is perfectly on-topic. However...
    ....you might want to check what is inserting these weird characters into
    your code, and...
    ....you might want to read the FAQ, in particular question 4.8:
    <http://c-faq.com/ptrs/passptrinit.html>. The rest of that section, and
    the rest of the FAQ, is instructive, too.

    Richard
     
    Richard Bos, Feb 11, 2008
    #6
  7. Below is a modified copy of your program. Not everything that might be
    improved has been noted, but enough to suggest where your main problems
    might lie apart from not handling allocation failure and not freeing
    allocated memory. Consider especially the lines marked with /* $ */ and
    think about what is different from your code and why.

    There are several questions in the FAQ that are relevant to your code.
    You might want to start with <http://c-faq.com/ptrs/passptrinit.html>,
    "Question 4.8 Q: I have a function which accepts, and is supposed to
    initialize, a pointer:"


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

    typedef struct node
    {
    struct node *next;
    int data;
    } Node;


    Node *alloc_node(int n);

    void insert(Node ** head, int n); /* $ */

    int main()
    {
    Node *nod = NULL;
    insert(&nod, 4); /* $ */
    printf("%d\n", nod->data);
    printf("%10p\n", (void *) nod); /* $ */
    printf("%10p\n", (void *) nod->next); /* $ */
    return 0;
    }

    Node *alloc_node(int n)
    {
    Node *tmp;
    tmp = malloc(sizeof *tmp);
    if (tmp) {
    tmp->data = n;
    tmp->next = NULL;
    }
    return tmp;
    }

    void insert(Node ** head /* $ */ , int n)
    {
    Node *tmp;
    if (*head == NULL) { /* $ */
    *head = alloc_node(n); /* $ */
    }

    else {
    tmp = alloc_node(n);
    (*head)->next = tmp; /* $ */
    }
    }
     
    Martin Ambuhl, Feb 11, 2008
    #7
  8. Lars

    Ian Collins Guest

    Your client? They don't appear to be in the message source.
     
    Ian Collins, Feb 11, 2008
    #8
  9. Lars

    Lars Guest

    Thanks for the pointers :)
    Regards
    Lars

     
    Lars, Feb 11, 2008
    #9
  10. Lars

    Randy Howard Guest

    I saw them as well. I'm pretty sure it's some UTF/Unicode weirdness
    with tab expansion or perhaps newline encoding somewhere along the way,
    and we're both using different clients, on different operating systems.
    Being that the OP is apparently in Denmark, it's a safe bet that it
    something along those lines.
     
    Randy Howard, Feb 11, 2008
    #10
  11. Lars

    SM Ryan Guest

    # int main()
    # {
    # Node *nod = NULL;
    # insert(&nod, 4); /* $ */
    # printf("%d\n", nod->data);
    # printf("%10p\n", (void *) nod); /* $ */
    # printf("%10p\n", (void *) nod->next); /* $ */
    # return 0;
    # }

    # void insert(Node ** head /* $ */ , int n)
    # {
    # Node *tmp;
    # if (*head == NULL) { /* $ */
    # *head = alloc_node(n); /* $ */
    # }
    #
    # else {
    # tmp = alloc_node(n);
    # (*head)->next = tmp; /* $ */
    # }
    # }

    I prefer an applicative style. An equivalent to the above would be
    ...
    nod = insert(nod,4);
    ...
    Node *insert(Node * head, int n)
    {
    Node *tmp;
    if (head == NULL) { /* $ */
    head = alloc_node(n); /* $ */
    }
    else {
    tmp = alloc_node(n);
    head->next = tmp; /* $ */
    }
    return head;
    }

    There may be a marginal difference that an applicative style is
    easier for the compiler to optimise, but overall it is question
    of which is easier for you to program in.

    For example if you were to implement a stack, one might do pushes
    push(&stack,a);
    push(&stack,b);
    ...
    or
    stack = push(stack,a);
    stack = push(stack,b);
    ...
    or
    stack = push(push(push(stack,a),b),...);
     
    SM Ryan, Feb 11, 2008
    #11
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.