Help with linked list in C

Applications Programming
1

i have this code

    typedef struct client_list {
        char *client_name;
        struct client_list * next;
        int client_socket_fd;
    } client;
    client *current, *head; head = NULL;
    char *h="test";
    add_client(current, h, head, &client_socket_fd);
    printf(current->client_name);

when it prints, i get a segmentation fault, what could be wrong?

the function add_client is

void add_client(client *current, char *username, client *head, int *client_socket_fd) {
    current = (client *)malloc(sizeof(client)); 
    current->client_name = username;
    current->next = head;
    current->client_socket_fd = *client_socket_fd;
    head = current;
    current = head;
}
2

I think the problem is, that you only work on the malloc'ed memory locally within the function.
You give add_client a pointer of type client. This is given by call by value, i.e. assigning the pointer a newly malloc'ed memory will not assign this memory to the pointer in the main function! Therefore your client *current is pointing to an undefined memory location even after call of add_client.

Better do this:

client *add_client(char *username, client *head, int *client_socket_fd) {
    client *current;
    current = (client *)malloc(sizeof(client)); 
    current->client_name = username;
    current->next = head;
    current->client_socket_fd = *client_socket_fd;
    return current;
}

Furthermore: What are you willing to achieve with:
head = current;
current = head
?

3

ok i changed it to

    typedef struct client_list {         
        char *client_name;         
        struct client_list * next;         
        int client_socket_fd;     
    } client;
    client *current; current->next = NULL;
    current = add_client("test", current, &client_socket_fd);
    printf(current->client_name);

and still segmentation error...

client *add_client(char *username, client *head, int *client_socket_fd) {
    client *current = (client *)malloc(sizeof(client)); 
    current->client_name = username;
    current->next = head;
    current->client_socket_fd = *client_socket_fd;
    return current;
}
4 
client *current;

This is just a pointer. It doesn't point to anything yet. The very next thing you do is use this invalid pointer: current->next = NULL; That could be trying to write to any memory whatsoever.

Do current=NULL; instead. A NULL head will mean an empty list. If you follow the logic in your function, it still works fine when head is NULL -- 'next' will be set to NULL, meaning end of list, and it won't crash because nothing uses the contents of 'head'.

5

if this makes a difference, the way i am doing the list is backwards, so when i want to add something to it, i create a new one in the function, then add the given one (which could be NULL or an actual list) to the next, then return the new one

6

To be honest, I did not understand your last sentence.

Anyways it does not make much difference whether you want a new entry to be the first one (after the head) or the last one. It is all just rearranged the next pointer. In the first case you rearrange the pointer from the head element to your newly created one and from there to the previously first, in the latter case you just traverse the list and change the last pointer form NULL to the new entry, making sure that the pointer in the newly created element points to NULL

7

i still don't follow what your saying....

8

Linked List Basics

This should be a really detailed explanation of linked lists, code is written in C as well

9

It's not that. Your very first line of code crashed it -- it never had a chance to crash anywhere else :smiley:

Your linked list function should actually work if you just set the first node to NULL. You don't need pointers to pass client_socket_fd though, just a plain int will be fine.

10

ok i got it to work now
i did

    client *current;
    current = (client *)malloc(sizeof(client));
    current->next=NULL;
    current = add_client("tttttttt", current, &client_socket_fd);
    current = add_client("ttttttttt", current, &client_socket_fd);
    printf("HERE\n");
    write(1,current->client_name,9);
    printf("\n");
    write(1,current->next->client_name,8);
    printf("\nHERE\n");
    

client *add_client(char *username, client *head, int *client_socket_fd) {
    client *current = (client *)malloc(sizeof(client)); 
    current->client_name = username;
    current->client_socket_fd = *client_socket_fd;
    current->next = head;
    return current;
}

and the results are

HERE
ttttttttt
tttttttt
HERE

i dont suppose theres anything still wrong here?

---------- Post updated at 04:23 PM ---------- Previous update was at 04:17 PM ----------

hmm, the above is ok, but when i use it with this function

int already_there(client *current, char *username) {
    
    while(current) {
        if (strcmp(current->client_name, username)==0) return(1);
        current = current->next;
    }

    return(0);
}

i get a segmentation fault...

11

That's wrong. Now there's no way to tell when the list's empty, and have a garbage blank node at the end of your list that might print garbage or crash.

Could you try what I suggested before instead? Sometimes it feels like I'm talking to a wall here :wall:

12

ok so now i have

    client *current, *head=NULL;
    current = add_client("tttttttt", current, client_socket_fd);
    current = add_client("ttttttttt", current, client_socket_fd);
    printf("HERE\n");
    write(1,current->client_name,9);
    printf("\n");
    write(1,current->next->client_name,8);
    printf("\nHERE\n");

i dont understand how i am supposed to use head here

client *add_client(char *username, client *head, int client_socket_fd) {
    client *current = (client *)malloc(sizeof(client)); 
    current->client_name = username;
    current->client_socket_fd = client_socket_fd;
    current->next = head;
    return current;
}
13

The exact same way you were using it in the first place. Pointers don't do anything mystical. If you pass NULL into your add_client function, you'll get NULL in your add_client function. When you set current->next to it, you're setting it to NULL.

They're just variables. You can copy them around just like you would a float or an int or a structure. The only time pointers do anything special is when you use their contents with the -> operator, the * operator, or the [] operator.

14

the use of head was from an example, but i don't understand why its being done, and why my function isn't working the same way (without using head)

15

a NULL pointer means end-of-list, right? So an empty list is nothing but a NULL pointer.

You weren't doing the same thing because you allocated and used memory when I didn't. I'll try again to illustrate why you don't need to allocate memory for the purpose of storing nothing:

# This isn't C code, I'm trying to show what pointers are pointing to what memory.
# The numbers are also totally made up.  They could be anything malloc()
# and your compiler give you.  The point is that pointers are just a special
# kind of number.

# What you did originally, declaring head without setting it to anything.
# When you tried to write to ????????????, the program crashed.
Address          Variable
0x100000000    head=0x????????????
0x??????????     INVALID MEMORY

# What I suggested instead.
# You still can't write to head->next since the pointer's invalid,
# but at least you know the memory's invalid.  You can use this
# to mean an empty list.
Address          Variable
NULL              INVALID MEMORY
0x100000000    head=NULL

# What a list with one node in it would look like.
# 0x100000010 would be an address you got from malloc().
# 0x000005000 might be the address the C compiler substitutes
# for a literal string like "quoted string".
Address           Variable
NULL               INVALID MEMORY
0x000005000    "client name"
0x100000000    head=0x100000010
0x100000010    client_name=0x000005000, next=NULL, fd=5

# What a list with two nodes in it would look like.
# 0x100000010 and 0x100000020 would be addresses you got from malloc().
Address           Variable
NULL               INVALID MEMORY
0x000005000    "client name"
0x000005010    "client 2"
0x100000000    head=0x100000010
0x100000010    client_name=0x000005000, next=0x100000020, fd=5
0x100000020    client_name=0x000005010, next=NULL, fd=6

Pointers never hold anything except an address -- a number meaning a place in memory.

When you left head unset, it ended up as some garbage ???????????. When you try to write anything inside ????????????, your computer knows you don't have access to ???????????? and kills your program.

NULL is just a zero. Your program never has access to memory address zero so C programs use it as a marker that means "this pointer doesn't go anywhere". So an empty linked list, having no nodes at all, is a head pointer that goes nowhere.

So when you feed a NULL head into your add_client program, you're just feeding it a zero. And when you feed another node into your add_client program instead, you're just giving it a 0x100000010 or whatever. The pointer doesn't do anything but hold that number. What's different is how you use these numbers.

16

i tried changing it to

    client *current, *head=NULL;
    current = add_client("tttttttt", current, head, client_socket_fd);
    current = add_client("ttttttttt", current, head, client_socket_fd);
    printf("HERE\n");
    write(1,current->client_name,9);
    //printf("\n");
    //write(1,current->next->client_name,8);
    printf("\nHERE\n");

client *add_client(char *username, client *client_list, client *head, int client_socket_fd) {
    client *new_client = (client *)malloc(sizeof(client)); 
    new_client->client_name = username;
    new_client->client_socket_fd = client_socket_fd;
    new_client->next = head;
    head=new_client;
    client_list->next=head;
    return client_list;
}

and still segmentation fault

edit: nvm i think i see why this is wrong

17

Stop flailing around and read what I'm writing for you. You don't understand how pointers work yet, until you do, writing code that works is going to be a matter of chance.

Trying what I suggested for you pages ago would also be nice.

I'll put it all together for you even.

    typedef struct client_list {         
        char *client_name;         
        struct client_list * next;         
        int client_socket_fd;     
    } client;
    client *current=NULL;
    int client_socket_fd=3;
    current = add_client("test", current, client_socket_fd);
    printf(current->client_name);

client *add_client(char *username, client *head, int client_socket_fd) {
    client *current = (client *)malloc(sizeof(client)); 
    current->client_name = username;
    current->next = head;
    current->client_socket_fd = client_socket_fd;
    return current;
}
18

ok i think i got it working now with

    client *current;
    current = add_client("tttttttt", current, client_socket_fd);
    current = add_client("ttttttttt", current, client_socket_fd);
    printf("HERE\n");
    write(1,current->client_name,9);
    printf("\n");
    write(1,current->next->client_name,8);
    printf("\nHERE\n");

client *add_client(char *username, client *client_list, int client_socket_fd) {
    client *new_client = (client *)malloc(sizeof(client)); 
    new_client->client_name = username;
    new_client->client_socket_fd = client_socket_fd;
    new_client->next = client_list;
    return new_client;
}

do you see anything still wrong?

19

:wall:

   client *current;
    current = add_client("tttttttt", current, client_socket_fd);

# What you did originally, declaring head without setting it to anything.
# When you tried to write to ????????????, the program crashed.
Address          Variable
0x100000000    current=0x????????????
0x??????????     INVALID MEMORY

# What your last post does
Address          Variable
0x000005000    "ttttttttt"
0x000005010    "tttttttttt"
0x100000000    current=0x100000010
0x100000010    client_name=0x000005000, next=0x100000010, fd=9
0x100000020    client_name=0x000005010, next=??????????, fd=8
0x??????????     INVALID MEMORY

If you try to read beyond the second node, you'll use the invalid memory ??????????? and it will crash. Because it's not NULL, you can't even tell that it's invalid. You want this:

# What your last post does
Address          Variable
NULL              INVALID MEMORY
0x000005000    "ttttttttt"
0x000005010    "tttttttttt"
0x100000000    current=0x100000010
0x100000010    client_name=0x000005000, next=0x100000010, fd=9
0x100000020    client_name=0x000005010, next=NULL, fd=8

And you accomplish that with client *current=NULL;

20

well using the code i just put above, i got
HERE
ttttttttt
tttttttt
HERE
as the output, but when i use this function

int already_there(client *current, char *username) {
    //printf("OK\n");
    while(current) {
        if (strcmp(current->client_name, username)==0) return(1);
        //if (current->next==NULL) break;
        //else 
        current = current->next;
    }
    return(0);
}

it doesnt crash, but i dont get the right result