CODEWITHSUNDEEP
c
user1390162
user1390162

Reputation:

Trying to add a node into a linked list

I've been trying to add a new node into a linked list of profiles (for ex. facebook profiles), and I'm getting a runtime error while launching. This is what I got :

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

typedef struct friend {
    char *name;
    int age;
    char gender;
    struct friend* next;
} friend;

void node_add(friend* new);

int main(int argc, char *argv[]) {
    friend amit;
    friend *new;

    amit.name = "amit";
    amit.age = 16;
    amit.gender = 'm';

    node_add(new);
    new->name = "amit";

    printf ("name: %s\n", new->name);
    system("PAUSE");    

    return 0;
}

void node_add(friend* new) {
    new = (friend* )malloc(sizeof(friend));
    friend* head = new;
    new -> next = head;
}

I'm trying now to create a delete node function. I tried to find which node does the user wants to delete, and then delete it by doing

delete -> next = delete -> next -> next

The problem is, I need to get for the first node in the list. Here is what I wrote:

void node_delete(friend* delete) {
    friend *temp;
    char name[256];
    int i = 0, j = 0;

    printf ("Please enter the friend's name you want to delete: \n");
    fgets (name, 256, stdin);
    fgets (name, 256, stdin);

    while (0 == (strcmp(temp -> next -> name, delete -> next -> name))) {
        temp = friend -> next;
    }
    temp -> next = temp -> next -> next;
    free (delete);
}

Upvotes: 2

Views: 399

Answers (5)

Williham Totland
Williham Totland

Reputation: 28999

Edit:

It seems my test was a mite too quick, because there is in fact a pretty serious problem with this code, but it's subtle:

In main() you are never actually pointing new at anything. It's just a garbled pointer out into memory space, which might sometimes work, and most of the time, is just terrible.

friend *new; // here's your problem; change this to:
friend *new = malloc(sizeof(friend));

Also, never cast the results of malloc.

Reedit:

How a very simple linked list implementation might look:

typedef struct _node node;
struct _node {
  void *payload;
  node *next;
};

node *create_node () {
  node *retval = malloc(sizeof(node));

  retval->payload = NULL;
  retval->next = NULL;

  return retval;
}

node *add_node (node *target) {
  if (target->next)
    return;

  node *next = create_node();
  node->next = next;
}

node *node_search (node *haystack, void *needle) {
  while (haystack) {
    if (!compare(needle, haystack->payload)) {
      return haystack;
    } else {
      haystack = haystack->next;
    }
  }

  return NULL;
}

Implementation of deletion and insertion are left as an exercise to the reader.

Upvotes: 1

Sachin Mhetre
Sachin Mhetre

Reputation: 4543

You should have friend *head global.

And in the 

void node_add(friend* new) 
{
    new = (friend* )malloc(sizeof(friend));
    new->next = head;
    head = new; 
}

Upvotes: 1

Adarsh
Adarsh

Reputation: 78

The issue is in the following line: amit.name = "amit";

You should be a malloc and doing a strcpy()

Upvotes: -1

md5
md5

Reputation: 23699

You should use a double-pointer.

void node_add(friend **new) {
    *new = malloc(sizeof(friend));
    /* etc */
}

Upvotes: 0

KAction
KAction

Reputation: 2017

Of course, you alloc memory and assign it to local variable. If you want to change pointer, pass pointer with one more asterix. And, by the way, do not name anything like friend or new. Its keywords in C++, and it create not needed problems.

Upvotes: 1

Related Questions