error subscripted value is neither an array nor pointer nor vector

im trying to get my head around linked lists and its completely mystifying me my test code wont even compile i get the above error on all the parameters of the second printf and a warning about incompatible pointer type when i try to make the next point to the first element (copied straight out of the book)

Code:

---

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

typedef struct
{
    int door_number;
    char *p_street_name;
    char *p_city;
    struct Address *next;
}Address;

int main()
{
    Address *first = NULL;
    Address *new_address;
    Address first_address;


    first_address.door_number = 4;
    first_address.p_street_name = "some street";
    first_address.p_city = "some town";
    printf("door number = %d street = %s city = %s\n", first_address.door_number,
                                first_address.p_street_name, first_address.p_city);

    new_address = malloc(sizeof(Address));
    if (!new_address)
    {
        return 1;
    }
    new_address->door_number = 2;
    new_address->p_street_name = "some other street";
    new_address->p_city = "some other town";
    new_address->next = first;
    first = new_address;

    printf("door number = %d street = %s city = %s\n", first_address[1].door_number,
                                first_address[1].p_street_name, first_address[1].p_city);

    free(first);
    free(new_address);

    return 0;
}

---

coop

I don't know... maybe because it is neither an array, nor a pointer? Who knows?

Quote:

---

Originally Posted by flp1969

I don't know... maybe because it is neither an array, nor a pointer? Who knows?

---

i certainly don't as im knew to this so rather than being sarcastic because you couldn't come up with some convoluted way of doing it either with bitwise operators or double pointers to re allocated memory in some obscurely written function how about not wasting the effort of typing out some meaningless unhelpful drivel and point me in the right direction

Quote:

---

Originally Posted by cooper1200

i certainly don't as im knew to this so rather than being sarcastic because you couldn't come up with some convoluted way of doing it either with bitwise operators or double pointers to re allocated memory in some obscurely written function how about not wasting the effort of typing out some meaningless unhelpful drivel and point me in the right direction

---

Ok... there are 3 errors in lines 36-37 and another one in lines 39-40... good luck.

Quote:

---

Originally Posted by cooper1200

Address *first = NULL;
Address *new_address;
Address first_address;

---

First look at how you're declaring each variable and notice the difference between what they create, another thing is that you're calling free() on the same pointer twice, I'm surprised your app doesn't crash at that point

And don't forget about the error on line 33 as well.


Hint: For your printf() problems, look closely at line 16.

Quote:

---

I'm surprised your app doesn't crash at that point

---

Why are you surprised? The code doesn't compile so how can it crash?

aswdert it doesn't crash at any point because it doesn't compile

Quote:

---

Originally Posted by cooper1200

aswdert it doesn't crash at any point because it doesn't compile

---

Forgot about that, also might wanna stop using auto complete until you understand your error before trying to free the pointer

Quote:

---

Originally Posted by flp1969

Ok... there are 3 errors in lines 36-37 and another one in lines 39-40... good luck.

---

yet another comment with no useful information i already know where the errors are as i detailed in my original post

Quote:

---

Originally Posted by jimblumberg

And don't forget about the error on line 33 as well.


Hint: For your printf() problems, look closely at line 16.

Why are you surprised? The code doesn't compile so how can it crash?

---

i appreciate that i only declare one instance of the struct Address and call it first address. however i then create (or at least i intend to) a new instance of Address and call it new address. As i now have two instances of Address surely that is equivalent to writing first_address[2]

and im not surprised by it not crashing that was in response to someone else's comment saying they were surprised it doesn't crashc.
coop

Quote:

---

Originally Posted by cooper1200

yet another comment with no useful information i already know where the errors are as i detailed in my original post

---

I suggest you start by moving the code that fills each variable into their own functions e.g

Code:

---

Address FillAddress1(void);
Address FillAddress2(void);
Address FillAddress3(void);

---

You then will have an easier time spotting the problem because each of these functions will fill their own local variable and return that on the stack or wherever the compiler puts it, each variable in main() can then be filled like this depending on how it is declared

Code:

---

one = FillAddress1();
*two = FillAddress2();
three[0] = FillAddress3();

---

Quote:

---

i appreciate that i only declare one instance of the struct Address and call it first address. however i then create (or at least i intend to) a new instance of Address and call it new address.

---

Okay. But do you realize that that "new instance" is a totally different entity?

Quote:

---

As i now have two instances of Address surely that is equivalent to writing first_address[2]

---

Surely not. A single instance is not an array.

ok i have had another bash at it.

Code:

---

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

typedef struct
{
    int door_number;
    char *p_street_name;
    char *p_city;
    struct Address *next;
}Address;

int main()
{
    Address *first = NULL;
    Address *new_address = NULL;
    Address *t;
    new_address = malloc(sizeof(Address));

    if (!new_address)
    {
        return 1;
    }
    new_address->door_number = 3;
    new_address->p_street_name = "some street";
    new_address->p_city = "some town";
    new_address->next = NULL;

    first = new_address;

    new_address = malloc(sizeof(Address));

    if (!new_address)
    {
        return 1;
    }
    new_address->door_number = 3;
    new_address->p_street_name = "some other street";
    new_address->p_city = "some other town";
    new_address->next = NULL;
    first->next = new_address;

    t = first;
    while (t)
    {
        printf("door number = %d street = %s city = %s\n", t->door_number, t->p_street_name, t->p_city);
        t = t->next;
    }

    free(new_address);
    free(first);

    return 0;
}

---

im now even more confused i have 2 warnings about incompatible pointer type lines 40 and 46. which is baffleing me as line 29 works (at least no errors or warnings)

furthermore dot com i fail to see what this achieves as i now cant add any more "elements" as first points to the first instance and first->next is intended to point to the next instance (with the warning message it clearly doesn't). but after that its useless as i have no way of adding another node unless i declare another pointer that points at the second node so i can add a third and another pointer that points at the third instance so i can add a 4th etc etc. all this is done away with simply creating an array of the stuct in the first place as everything is hard coded.

Quote:

---

Originally Posted by jimblumberg

Okay. But do you realize that that "new instance" is a totally different entity?


Surely not. A single instance is not an array.

---

maybe this is where i am getting confused.

if i wrote Address[2] i have an array of 2 structs called address and could store 2 different sets of information.
if i understand what your saying correctly that is different to having a single deceleration of Address and dynamically creating another even though the net result is the same ie i can store two different sets of information.
coop

Code:

---

//            missing
//            __tag__
typedef struct Address
{
  int door_number;
  char *p_street_name;
  char *p_city;
  struct Address *next;
} Address;

---

As I said a single instance is a single instance not an array.

If you want to dynamically allocate an array of some type you need allocate enough memory for the number of elements you want.

For example to allocate an array of int of size 4 you would do something like:

Code:

---

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

int main(void)
{
    int *dynamic;

    dynamic = malloc(sizeof(int) * 4);  // Allocate an array with 4 elements

    // Assign some "random" values to the elements.
    dynamic[0] = 4;
    dynamic[1] = 2343;
    dynamic[2] = 36;
    dynamic[3] = 45;

    // Print the array contents.
    for(int i = 0; i < 4; ++i)
        printf("%d ", dynamic[i]);

    printf("\n");

    return 0;
}

---

Here is a more through example showing how to work with dynamically allocated single instances and using the structure.

Code:

---

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

typedef struct
{
    int door_number;
    char *p_street_name;
    char *p_city;
    struct Address *next;
}Address;

int main(void)
{
    int *dynamic;

    dynamic = malloc(sizeof(int) * 4);

    dynamic[0] = 4;
    dynamic[1] = 2343;
    dynamic[2] = 36;
    dynamic[3] = 45;

    for(int i = 0; i < 4; ++i)
        printf("%d ", dynamic[i]);

    printf("\n");

    // Single instance:
    int *single;
    single = malloc(sizeof(int)); // Dynamically allocate a single instance.

    // Assign a value to this instance.
    *single = 5;

    printf("%d\n", *single);

    // Now dynamically allocate a single instance of your structure:
    Address *single_structure;
    single_structure = malloc(sizeof(Address));
    single_structure->door_number = 21343;

    printf("%d\n", single_structure->door_number);

    // Now an array of that structure.
    Address *multiple;
    multiple = malloc(sizeof(Address) * 2);
    multiple[0].door_number = 123;
    multiple[1].door_number = 345;

    for(int i = 0; i < 2; ++i)
        printf("%d ", multiple[i].door_number);

    printf("\n");


    return 0;
}

---

To use those "strings" you will still need to either statically or dynamically allocate memory for them before you try to use them.

following flp's suggestion of putting in the missing tag i got the program to compile with no errors and run however declaring an extra pointer to keep track of where we are on the list misses an element(s) out (1 in my case but having played about i susspect that if i had 5 calls it would still only print out the first 2 and the last one. here is the code

Code:

---

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

typedef struct Address
{
    int door_number;
    char *p_street_name;
    char *p_city;
    struct Address *next;
}Address;

int main()
{
    Address *first = NULL;
    Address *new_address = NULL;
    Address *next_instance = NULL;
    Address *t;

    new_address = malloc(sizeof(Address));
    if (!new_address)
    {
        printf("error in memory allicating memory!");
        return 1;
    }
    new_address->door_number = 3;
    new_address->p_street_name = "some street";
    new_address->p_city = "some town";
    new_address->next = NULL;

    first = new_address;
    next_instance = new_address;

    new_address = malloc(sizeof(Address));

    if (!new_address)
    {
        printf("error in memory allicating memory!");
        goto clear_memory;
    }
    new_address->door_number = 5;
    new_address->p_street_name = "some other street";
    new_address->p_city = "some other town";
    new_address->next = NULL;
    first->next = new_address;
    next_instance = new_address;

    new_address = malloc(sizeof(Address));

    if (!new_address)
    {
        printf("error in memory allicating memory!");
        goto clear_memory;
    }
    new_address->door_number = 5;
    new_address->p_street_name = "another street";
    new_address->p_city = "jhkhhkhhkhkhjk town";
    new_address->next = NULL;
    next_instance->next = new_address;

    new_address = malloc(sizeof(Address));

    if (!new_address)
    {
        printf("error in memory allicating memory!");
        goto clear_memory;
    }
    new_address->door_number = 20;
    new_address->p_street_name = "yet another street";
    new_address->p_city = "yet another town";
    new_address->next = NULL;
    next_instance->next = new_address;

    t = first;
    while (t)
    {
        printf("door number = %d street = %s city = %s\n", t->door_number, t->p_street_name, t->p_city);
        t = t->next;
    }

    t = first;
clear_memory:

    while (t)
    {
        next_instance = t->next;
        printf("freeing\n");
        free(t);
        t = next_instance;
    }

    return 0;
}

---

coop

Quote:

---

Originally Posted by cooper1200

following flp's suggestion of putting in the missing tag i got the program to compile with no errors and run however declaring an extra pointer to keep track of where we are on the list misses an element(s) out (1 in my case but having played about i susspect that if i had 5 calls it would still only print out the first 2 and the last one. here is the code

Code:

---

**Snip**

---

coop

---

Make another struct that keeps track of the list

Code:

---

struct linkedList
{
 

  struct Address *Head;
  int count; // does come in handy...

};

---

And then make functions that pass this in, such as initialize, addToStart, addAfter, addBefore, addEnd, delete/AtStart/After/Before/AtEnd, distroyList, printList, orderBy, cloneList, findAddressByName, findAddressBy…

sorry i don't understand what you mean. if i have a struct that has a member pointing to first and a second member counting the elements in the list how do i move past the second element of the list as i have no reference to it.

ok this is my understanding of what i am trying to do (i hope it keeps the formatting)

Code:

---

/*
          *new_p = malloc.....
          *first = new_p
          *current = first
            head node
          ---------------
          |      |      |
 *first ->| data |*point| --->null
          |      |      |
          ---------------
                ^
                |
          *current
         
as there is only one item (the head) in the list both first and
current point to the head of the list.

          *new_p = malloc.....
          *first->next = new_p
          *current = new_p
                head                    2nd element
          ---------------              ---------------
          |      |      | first->next  |      |      |
 *first ->| data |*point|------------->| data |*point|-->null
          |      |      |              |      |      |
          ---------------              ---------------
                                              ^
                                              |
                                      *current

now there are two elements *first points to the 1st (the head) and both first->next
and *current point to the second element.

          *new_p = malloc.....
          *current->next = new_p
          *current = new_p

              head                      2nd element                    3rd element
          ---------------              ---------------                ---------------
          |      |      | first->next  |      |      | current->next  |      |      |
 *first ->| data |*point|------------->| data |*point|--------------->| data |*point|-->null
          |      |      |              |      |      |                |      |      |
          ---------------              ---------------                ---------------
                                                                            ^
                                                                            |
                                                                      *current
                                                                     
first still points to the head of the list, first->next points to the 2nd element
current->next points to the 3rd element and then current is updated to point at the 3rd
element.  more and more nodes can be added by setting current->next to the new address and
current to point at the new element.
*/

---

coop

Traditionally, head is used to name a pointer to the first node of the linked list, tail is used to name a pointer to the last node of the linked list, and if the name is used, then current is used to name a pointer to the current node when you're iterating over the linked list.

here is my effort to put the above into code

Code:

---

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

typedef struct Address
{
    int door_number;
    char *street;
    char *city;
    struct Address *next;
}Address;

void add_address(Address *first, Address *current);
void print_list(Address *first);
void delete_list(Address *first);

int main()
{
    Address *first = NULL, *current = NULL;

    add_address(first, current);
    add_address(first, current);
    add_address(first, current);
    add_address(first, current);
    add_address(first, current);

    print_list(first);
    delete_list(first);

    return 0;
}
void add_address(Address *first, Address *current)
{
    char my_str[20];
    Address *p_new_element;

    //get a new block of memmory and test the pointer isnt null
    p_new_element = malloc(sizeof(Address));
    if (!p_new_element)
    {
        printf("memmory allocation error");
        return;
    }
    //get door number and assign it to the new elements member 'door_number'
    printf("Please enter the door number: ");
    scanf(" %d", &p_new_element->door_number);

    //get street name and assign it to my_str
    printf("Please enter street name: ");
    scanf(" %s", my_str);
    //create memory for the street name and assign the pointer address->street to point to it
    p_new_element->street = malloc(strlen(my_str) + 1);
    //copy contents of my_str into the memmory location pointed to by address->street
    strcpy(p_new_element->street, my_str);

    //get city name
    printf("Please enter city name: ");
    scanf(" %s", my_str);
    //create memory for the city name and assign the pointer address->city to point to it
    p_new_element->city = malloc(strlen(my_str) + 1);
    //copy contents of my_str into the memmory location pointed to by address->city
    strcpy(p_new_element->city, my_str);

    if (first == NULL) // no list yet so make p_new_element the head
    {
        first = p_new_element;
        first->next = NULL;
        current = first;
        return;
    }
    if (first->next == NULL) //only one element in the list
    {
        first->next = p_new_element;
        current = p_new_element;
        current->next = NULL;
        return;
    }
    //more than 2 elements
    current->next = p_new_element;
    current = p_new_element;
    current->next = NULL;
}

void print_list(Address *first)
{
    int count = 1;
    Address *temp = first;

    while (temp)
    {
        printf("address no %d is door number %d street %s city %s\n", count, temp->door_number,
                                                                      temp->street, temp->city);
        count++;
        temp = temp->next;
    }
}

void delete_list(Address *first)
{
    int count = 1;
    Address *temp = NULL;

    printf("freeing memmory....\n");
    while (first)
    {
        temp = first->next; // set temp to point at the next element in the list
        printf("freeing element %d's street name\n", count);
        free(first->street); //free the memmory block that holds firsts street name
        printf("freeing element %d's city name\n", count);
        free(first->city); //free the memmory block that holds firsts city name
        printf("freeing struct %d's memmory\n", count);
        free(first); //free the memmory block that holds the first element
        first = temp; // set first to point at the next memmory block;
        count++;
    }
    printf("finished freeing all allocated memmory\n");
}

---

of course it doesn't work
i input 5 lots of addresses then it doesn't print any of them then says freeing memory and then jumps straight to memory freed. on stepping through the code first is never changed from being set to null despite add address being passed a pointer to first.
coop

the more i try this the more confused im getting i just don't get the point of this at all. yes it saves memory by only having the number of entries you require and can be fluid however7
{

Code:

---

int count = 0
Address *multiple;

multiple = malloc(sizeof(Address));
count++;

//need more
multiple = realloc(multiple, sizeof(Address) * (count + 1));
count++

---

achieves the same thing.
coop

Quote:

---

Originally Posted by cooper1200

the more i try this the more confused im getting i just don't get the point of this at all. yes it saves memory by only having the number of entries you require and can be fluid however7
{

Code:

---

int count = 0
Address *multiple;

multiple = malloc(sizeof(Address));
count++;

//need more
multiple = realloc(multiple, sizeof(Address) * (count + 1));
count++

---

achieves the same thing.
coop

---

Drop the count + 1 and leave just count in there, also initialise count to 1, if the allocation fails the decrement it, 9 times out of 10 you'll never reach the decrement and save yourself an operation when repeating this say 1000 times

thanks.... is this bad programming practice then??? why have the facility of linked lists if a thicko like me can come up with a simple way round them
coop

ok another attempt at getting linked lists to work. the wind was blowing from the east this time so i tried passing the address of *first to the function

Code:

---

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

typedef struct Address
{
    int door_number;
    char *street;
    char *city;
    struct Address *next;
}Address;

void add_address(Address **first, Address **current);
void print_list(Address *first);
void delete_list(Address **first);

int main()
{
    Address *first = NULL, *current = NULL;

    add_address(&first, &current);
    add_address(&first, &current);
    add_address(&first, &current);
    //add_address(first, current);
    //add_address(first, current);

    print_list(first);
    delete_list(&first);

    return 0;
}
void add_address(Address **first, Address **current)
{
    char my_str[20];
    Address *p_new_element;

    //get a new block of memmory and test the pointer isnt null
    p_new_element = malloc(sizeof(Address));
    if (!p_new_element)
    {
        printf("memmory allocation error");
        return;
    }
    //get door number and assign it to the new elements member 'door_number'
    printf("Please enter the door number: ");
    scanf(" %d", &p_new_element->door_number);

    //get street name and assign it to my_str
    printf("Please enter street name: ");
    scanf(" %s", my_str);
    //create memory for the street name and assign the pointer address->street to point to it
    p_new_element->street = malloc(strlen(my_str) + 1);
    if (!p_new_element->street)
    {
        printf("error in getting memmory for street name");
        return;
    }
    //copy contents of my_str into the memmory location pointed to by address->street
    strcpy(p_new_element->street, my_str);

    //get city name
    printf("Please enter city name: ");
    scanf(" %s", my_str);
    //create memory for the city name and assign the pointer address->city to point to it
    p_new_element->city = malloc(strlen(my_str) + 1);
    if (!p_new_element->city)
    {
        printf("error in getting memmory for city name");
        return;
    }
    //copy contents of my_str into the memmory location pointed to by address->city
    strcpy(p_new_element->city, my_str);

    if (*first == NULL) // no list yet so make p_new_element the head
    {
        *first = p_new_element;
        *first->next = NULL;
        *current = *first;
        return;
    }
    if (*first->next == NULL) //only one element in the list
    {
        *first->next = p_new_element;
        *current = p_new_element;
        *current->next = NULL;
        return;
    }
    //more than 2 elements
    *current->next = p_new_element;
    *current = p_new_element;
    *current->next = NULL;
}

void print_list(Address *first)
{
    int count = 1;
    Address *temp = first;

    while (temp)
    {
        printf("address no %d is door number %d street %s city %s\n", count, temp->door_number,
                                                                      temp->street, temp->city);
        count++;
        temp = temp->next;
    }
}

void delete_list(Address **first)
{
    int count = 1;
    Address *temp = NULL;

    printf("freeing memmory....\n");
    while (*first)
    {
        temp = *first->next; // set temp to point at the next element in the list
        printf("freeing element %d's street name\n", count);
        free(*first->street); //free the memmory block that holds firsts street name
        printf("freeing element %d's city name\n", count);
        free(*first->city); //free the memmory block that holds firsts city name
        printf("freeing struct %d's memmory\n", count);
        free(*first); //free the memmory block that holds the first element
        *first = temp; // set first to point at the next memmory block;
        count++;
    }
    printf("finished freeing all allocated memmory\n");
}

---

this produces 9 errors lines: 77 81 83 85 89 91 116 118 120; all saying error first is a pointer did you mean ->. well yes that's why i used the -> operator duh!! i guess the wind changed again without me noticing
coop

First point, since I posting from a phone I can't tell what lines those are exactly but I'll assume they're down at the last function where you try to use a pointer (first) to another pointer or list of pointers, in that scenario you must enclose *first in brackets or change it for first[0] otherwise the compiler will think your referencing first 1st before de-referencing the contents of first when you want it the other way round, de-referencing first then referencing the contents,
2nd point as I mentioned above leave filling to another function like this:

Code:

---

Address useraddr() { Address tmp; scanf(...);... return tmp;}

---

then your variables can be automatically filled properly by the compiler instead when you assign the result to them

Quote:

---

Originally Posted by cooper1200

thanks.... is this bad programming practice then??? why have the facility of linked lists if a thicko like me can come up with a simple way round them
coop

---

Simply put the more operations the longer a task takes, best practice is to always keep an eye out for ways to shorten the time, that way when you get to time critical functions you'll have already developed the necessary habit and trained your brain to spot most unnecessary operations or poorly used operations, as for linked lists there are cases for it such as trees but yes it doesn't fit everything.

ok finally using awsdert's suggestion of replacing *first with (*first) worked. here is the code

Code:

---

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

typedef struct Address
{
    int door_number;
    char *street;
    char *city;
    struct Address *next;
}Address;

void add_address(Address **first, Address **current);
void print_list(Address *first);
void delete_list(Address **first);

int main()
{
    Address *first = NULL, *current = NULL;

    add_address(&first, &current);
    add_address(&first, &current);
    add_address(&first, &current);
    //add_address(first, current);
    //add_address(first, current);

    print_list(first);
    delete_list(&first);

    return 0;
}

void add_address(Address **first, Address **current)
{
    char my_str[20];
    Address *p_new_element;

    //get a new block of memmory and test the pointer isnt null
    p_new_element = malloc(sizeof(Address));
    if (!p_new_element)
    {
        printf("memmory allocation error");
        return;
    }

  //get door number and assign it to the new elements member 'door_number'
    printf("Please enter the door number: ");
    scanf(" %d", &p_new_element->door_number);

    //get street name and assign it to my_str
    printf("Please enter street name: ");
    scanf(" %s", my_str);
    //create memory for the street name and assign the pointer address->street to point to it
    p_new_element->street = malloc(strlen(my_str) + 1);
    if (!p_new_element->street)
    {
        printf("error in getting memmory for street name");
        return;
    }
    //copy contents of my_str into the memmory location pointed to by address->street
    strcpy(p_new_element->street, my_str);

    //get city name
    printf("Please enter city name: ");
    scanf(" %s", my_str);
    //create memory for the city name and assign the pointer address->city to point to it
    p_new_element->city = malloc(strlen(my_str) + 1);
    if (!p_new_element->city)
    {
        printf("error in getting memmory for city name");
        return;
    }
    //copy contents of my_str into the memmory location pointed to by address->city
    strcpy(p_new_element->city, my_str);

    if (*first == NULL) // no list yet so make p_new_element the head
    {
        *first = p_new_element;
        (*first)->next = NULL;
        *current = *first;
        return;
    }
    if ((*first)->next == NULL) //only one element in the list
    {
        (*first)->next = p_new_element;
        *current = p_new_element;
        (*current)->next = NULL;
        return;
    }
    //more than 2 elements
    (*current)->next = p_new_element;
    *current = p_new_element;
    (*current)->next = NULL;
}

void print_list(Address *first)
{
    int count = 1;
    Address *temp = first;

    while (temp)
    {
        printf("address no %d is door number %d street %s city %s\n", count, temp->door_number,
                                                                      temp->street, temp->city);
        count++;
        temp = temp->next;
    }
}

void delete_list(Address **first)
{
    int count = 1;
    Address *temp = NULL;

    printf("freeing memmory....\n");
    while (*first)
    {
        temp = (*first)->next; // set temp to point at the next element in the list
        printf("freeing element %d's street name\n", count);
        free((*first)->street); //free the memmory block that holds firsts street name
        printf("freeing element %d's city name\n", count);
        free((*first)->city); //free the memmory block that holds firsts city name
        printf("freeing struct %d's memmory\n", count);
        free(*first); //free the memmory block that holds the first element
        (*first) = temp; // set first to point at the next memmory block;
        count++;
    }
    printf("finished freeing all allocated memmory\n");
}

---

however on this youtube video at approx 13 minuets he uses a double pointer and doesn't use the (*first) he used *first->next = null etc etc so why do i have to use (*first) and he didn't?
coop

Cause he either forgot and had non-compiling code or simply is a bad programmer talking out of his ..........

too be fair there were lots of oh hang on no it works like this statements as he was explaining it but it did compile. anyway my code works now i need to impliment your other suggeston and have a function that gets the data from the user then assigns it in one lump.
coop

Quote:

---

Originally Posted by cooper1200

too be fair there were lots of oh hang on no it works like this statements as he was explaining it but it did compile. anyway my code works now i need to impliment your other suggeston and have a function that gets the data from the user then assigns it in one lump.
coop

---

From the console scanf is the solution like in my above example, just use &tmp.VALUE for each expected value you put in the format parameter to use that right, for GUI I would suggest something like Tefaf's (I think that was the name) IUP Library

ok now whats up all i have done is switch the inputting of data to a separate function.

Code:

---

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

typedef struct Address
{
    int door_number;
    char *street;
    char *city;
    struct Address *next;
}Address;

void add_address(Address **first, Address **current);
Address get_address_details(void);
void print_list(Address *first);
void delete_list(Address **first);

int main()
{
    Address *first = NULL, *current = NULL;

    add_address(&first, &current);
    add_address(&first, &current);
    add_address(&first, &current);
    //add_address(first, current);
    //add_address(first, current);

    print_list(first);
    delete_list(&first);

    return 0;
}

void add_address(Address **first, Address **current)
{
    Address *p_new_element, temp;

    //get a new block of memmory and test the pointer isnt null
    p_new_element = malloc(sizeof(Address));
    if (!p_new_element)
    {
        printf("memmory allocation error");
        return;
    }

    temp = get_address_details();
    p_new_element = &temp;
    printf("door = %d street = %s city = %s\n", temp.door_number, temp.street, temp.city);
    printf("door = %d street = %s city = %s\n", p_new_element->door_number, p_new_element->street,
                                                                              p_new_element->city);
    if (*first == NULL) // no list yet so make p_new_element the head
    {
        *first = p_new_element;
        (*first)->next = NULL;
        *current = *first;
        return;
    }
    if ((*first)->next == NULL) //only one element in the list
    {
        (*first)->next = p_new_element;
        *current = p_new_element;
        (*current)->next = NULL;
        return;
    }
    //more than 2 elements
    (*current)->next = p_new_element;
    *current = p_new_element;
    (*current)->next = NULL;
}

Address get_address_details(void)
{
    char my_str[20];
    Address tmp;

    //get door number and assign it to the new elements member 'door_number'
    printf("Please enter the door number: ");
    scanf(" %d", &tmp.door_number);

    //get street name and assign it to my_str
    printf("Please enter street name: ");
    scanf(" %s", my_str);
    //create memory for the street name and assign the pointer address->street to point to it
    tmp.street = malloc(strlen(my_str) + 1);
    if (!tmp.street)
    {
        printf("error in getting memmory for street name");
        //return;
    }
    //copy contents of my_str into the memmory location pointed to by address->street
    strcpy(tmp.street, my_str);

    //get city name
    printf("Please enter city name: ");
    scanf(" %s", my_str);
    //create memory for the city name and assign the pointer address->city to point to it
    tmp.city = malloc(strlen(my_str) + 1);
    if (!tmp.city)
    {
        printf("error in getting memmory for city name");
        //return;
    }
    //copy contents of my_str into the memmory location pointed to by address->city
    strcpy(tmp.city, my_str);

    return tmp;
}

void print_list(Address *first)
{
    int count = 1;
    Address *temp = first;

    while (temp)
    {
        printf("address no %d is door number %d street %s city %s\n", count, temp->door_number,
                                                                      temp->street, temp->city);
        count++;
        temp = temp->next;
    }
}

void delete_list(Address **first)
{
    int count = 1;
    Address *temp = NULL;

    printf("freeing memmory....\n");
    while (*first)
    {
        temp = (*first)->next; // set temp to point at the next element in the list
        printf("freeing element %d's street name\n", count);
        free((*first)->street); //free the memmory block that holds firsts street name
        printf("freeing element %d's city name\n", count);
        free((*first)->city); //free the memmory block that holds firsts city name
        printf("freeing struct %d's memmory\n", count);
        free(*first); //free the memmory block that holds the first element
        (*first) = temp; // set first to point at the next memmory block;
        count++;
    }
    printf("finished freeing all allocated memmory\n");
}

---

it compiles and the two printf statements after assigning temp to the pointer display the info correctly as entered. but once i hit the print function it never breaks out and prints a lot of rubbish to the screen
coop

For starters set temp to 0 like this:

Code:

---

Address temp = {0};

---

when initialising l, second NEVER give a pointer to a stack variable to a pointer value being returned, at work so few opportunities to respond, and have short replies, I'll take another look when I get a chance

Sorry meant tmp in get_address_details(), still don't have time to do thorough check of your code however look at when you fill the "next" parameter and make sure the it's the previous address that recieves the pointer and don't tamper with the tmp one

ok god knows how this is happening and what the hell has caused it but it appears that when setting the head of the list the first->next doesn't get set to null; therefor the second if doesnt get run. and all other nodes are set on current so nothing is linked and there is no null to terminate the print loop.

You got a memory leak in add_address()

Here's an example of handing those pointers information

Code:

---

*p_new_address = get_address_details();
if ( p_prv_address) p_prv_address->next = p_new_address;

---

Also you should allocate all memory you're going to use BEFORE asking the user for their details

ok im not getting this.... i have done nothing other than move the data entry into another function and yet it suddenly changes how the program operates ie it doesn't work. the only call to malloc in add_address gets the lump of memory for the new node. i would of thought i need that until i call delete list and if i try to free it at the end of the function i get double free or corruption (out)
Aborted (core dumped)

Process returned 134 (0x86) execution time : 7.599 s
Press ENTER to continue.

i just don't understand how something that appears to work as far as the printf's in get address show suddenly stops a previously working section of code working.

coop

ok thanks for that it all works now.

Quote:

---

Originally Posted by awsdert

Also you should allocate all memory you're going to use BEFORE asking the user for their details

---

i dont know what the user is going to input it might be the full 20 characters or might just be 5 should i malloc for the full 20 then use realloc to reduce it to the size required?

Yep, from a user standpoint we don't want to give our details only to find an error message saying not enough memory, we just wasted our time as a result, better to give a smoother experience by saying not enough memory from the get go and skipping the message if we got the memory and just continuing normally

Quote:

---

Originally Posted by awsdert

Drop the count + 1 and leave just count in there, also initialise count to 1, if the allocation fails the decrement it, 9 times out of 10 you'll never reach the decrement and save yourself an operation when repeating this say 1000 times

---

While I agree with your analysis, I think that also misses the forest for the trees: cooper1200's example reallocates every time an element is inserted to the dynamic array. This potentially means quadratic time complexity if realloc somehow isn't able to expand the dynamic array in-place for a sufficiently large number of insertions (whereas a single additional addition on each insertion doesn't change the time complexity at all, and is a relatively small penalty in practice). Avoiding this requires more work, e.g., keeping track of both size and capacity and having a suitable reallocation strategy, so it isn't quite as simple as cooper1200 believes it to be (although it doesn't have to be too complicated either).

Quote:

---

Originally Posted by cooper1200

thanks.... is this bad programming practice then??? why have the facility of linked lists if a thicko like me can come up with a simple way round them

---

Try inserting into a dynamic array at the front. You'll find that it takes linear time. Try inserting into a linked list at the front. You'll find that it takes constant time. Linked list wins this round!

Given a pointer to somewhere in the middle of a dynamic array, try inserting an element after it. You'll find that it takes linear time. Given a pointer to a node in the middle of a linked list, try inserting an element after it. You'll find that it takes constant time. Linked list wins this round!

Given a pointer to somewhere in the middle of a dynamic array, try removing that element. If you want the removal to be stable, you'll find that it takes linear time. Try removing the next element in a linked list given a pointer to a node in the middle. You'll find that it takes constant time while maintaining stability. Linked list wins this round!

Try removing all the elements of a dynamic array that matches some criteria. If you implement a naive algorithm, you'll find that this takes quadratic time, whereas the obvious algorithm for a linked list takes linear time. You can implement a linear time algorithm to do this for a dynamic array, but it is more complex. Linked list wins this round!

You want to sort items fast while maintaining stability, so you implement a merge sort with a dynamic array. You find that it takes O(n) auxiliary space. You switch to using a linked list. You find that it takes O(1) auxiliary space. Linked list wins this round!

Of course, I have cherry picked examples in which linked lists have an advantage over dynamic arrays. I could also have cherry picked other examples in which dynamic arrays have an advantage over linked lists. So, as awsdert pointed out, "there are cases for it (...) but yes it doesn't fit everything". (Note that "such as trees" is incorrect: while linked lists can be seen as degraded trees, the use cases for trees differ from those of linked lists because there are ways to avoid such degradation, e.g., by using balanced binary trees.)

Click_here gave you good advice in post #19 that you seem to be ignoring (besides ignoring what I told you about naming conventions in post #22):

Quote:

---

Originally Posted by Click_here

Make another struct that keeps track of the list

---

Now, keeping track of the count has its pros and cons: it can mean extra work for little return if you rarely need the count, and for some operations it can change a constant time complexity to a linear time complexity because you may have to recount. However, it makes it much easier when you have both head and tail pointers, e.g.,

Code:

---

struct AddressList
{
    struct Address *head;
    struct Address *tail;
};

void add_address(AddressList *addresses)

---

I've gone and dug up some of the old singly linked list code that I have laying around :)

To get the most out of your LL, seporate your data type from your list by using a void*

Code:

---

typedef struct SList
{
    int Tag; // type of data
    struct sList *Next;
    void *Object;
    size_t Size;
}
SList;

---

This means that you can use the same add/remove/update for any data type :)


The advantages of using a linked list become apparent as you build upon it - There are a few problem with signly linked lists that are solved by using a double linked list (such as the "split list" problem where the code accidently loses where the start of the list is), but that is the next step...

You can "cut and paste" items/sections of the list, modify the list to create circular lists, create a stack, create a queue, priority queue, binary trees. You can add additional dimensions and create a sparse matrix, graphs, ...

You can create an "event" based programme where the object becomes function pointers...


All of these things are built upon a linked list and if you want to utilise them you need to start here!

Quote:

---

Originally Posted by laserlight

While I agree with your analysis, I think that also misses the forest for the trees: cooper1200's example reallocates every time an element is inserted to the dynamic array. This potentially means quadratic time complexity if realloc somehow isn't able to expand the dynamic array in-place for a sufficiently large number of insertions (whereas a single additional addition on each insertion doesn't change the time complexity at all, and is a relatively small penalty in practice). Avoiding this requires more work, e.g., keeping track of both size and capacity and having a suitable reallocation strategy, so it isn't quite as simple as cooper1200 believes it to be (although it doesn't have to be too complicated either).

---

Didn't think of that

Quote:

---

Originally Posted by laserlight

Given a pointer to somewhere in the middle of a dynamic array, try removing that element. If you want the removal to be stable, you'll find that it takes linear time. Try removing the next element in a linked list given a pointer to a node in the middle. You'll find that it takes constant time while maintaining stability. Linked list wins this round!

---

Code:

---

node->pos = -1

---

Something not linear about that? Could be recycled later, although a simple malloc() is indeed more efficient than looping through elements to find any unused ones before falling back to realloc

Quote:

---

Originally Posted by laserlight

Try removing all the elements of a dynamic array that matches some criteria. If you implement a naive algorithm, you'll find that this takes quadratic time, whereas the obvious algorithm for a linked list takes linear time. You can implement a linear time algorithm to do this for a dynamic array, but it is more complex. Linked list wins this round!

---

Code:

---

node->pos = -1

---

Something complex about this?

Quote:

---

Originally Posted by laserlight

Of course, I have cherry picked examples in which linked lists have an advantage over dynamic arrays. I could also have cherry picked other examples in which dynamic arrays have an advantage over linked lists. So, as awsdert pointed out, "there are cases for it (...) but yes it doesn't fit everything". (Note that "such as trees" is incorrect: while linked lists can be seen as degraded trees, the use cases for trees differ from those of linked lists because there are ways to avoid such degradation, e.g., by using balanced binary trees.)

---

Don't know what you mean here

Quote:

---

Originally Posted by laserlight

Click_here gave you good advice in post #19 that you seem to be ignoring (besides ignoring what I told you about naming conventions in post #22):

---

I agree his naming convention sucks hard, I might be bad but I do clean up on smaller functions and give more fore-thought on bigger functions ( or functions expected to be big)

Quote:

---

Originally Posted by laserlight

Now, keeping track of the count has its pros and cons: it can mean extra work for little return if you rarely need the count, and for some operations it can change a constant time complexity to a linear time complexity because you may have to recount. However, it makes it much easier when you have both head and tail pointers, e.g.,

Code:

---

struct AddressList
{
    struct Address *head;
    struct Address *tail;
};

void add_address(AddressList *addresses)

---

---

A struct is over the top in this case:

Code:

---

Address* ins_address( Address *tail, size_t count, Address **next ) {
  size_t i;
  Address *head;
  if ( !count ) return NULL;
  head = calloc( count, sizeof(Address) );
  if ( !head ) return NULL;
  head->count = count;
  --count;
  for ( i  = 0; i < count; ++i ) {
    head[i].next = &(head[i+1]);
  }
  if ( tail ) (
    head[count].next = tail.next;
    tail->next = head;
  }
  if ( next ) *next = head[count].next;
  return head;
}
Address* rem_address( Address *head, Address *tail ) {
  size_t i;
  Address *node, *next = head, *prev;
  if ( !head ) return NULL;
  prev = head->prev;
  while ( next && next != tail ) {
    node = next;
    if ( node->count ) {
      node->count--;
      for ( i = 0; i < node->count; ++i ) {
        next = &(node[i]);
        if ( next == tail ) break;
        next->next = NULL;
    }
    if ( next == tail ) break;
    next = node[node->count].next;
    node->count++;
    free( node );
    }
    else { next = node->next; }
  }
  // In case tail was NULL
  if ( tail ) {
    next = tail->next;
    if ( tail->count == 1 ) free(tail);
  }
  return next;
}
int main() {
  Address *head = ins_address( NULL, 1000, NULL ), *tail = NULL, *node;
  if ( !head ) return 1;
  tail = &(head[999]);
  // Not using the *->next parameter of tail
  (void)rem_address( head, NULL );
  return 0;
}

---

Quote:

---

Originally Posted by awsdert

Something not linear about that? Could be recycled later, although a simple malloc() is indeed more efficient than looping through elements to find any unused ones before falling back to realloc

---

I'm assuming you're presenting the scenario of an array of elements of a type that has some kind of designated invalid state. In that case, what you're doing is marking the object for removal; you haven't removed it from the dynamic array: until you actually remove them, you don't have the advantages of say, random access to valid elements, as it is no longer an array of valid elements. Same for removal of elements matching some criteria.

Quote:

---

Originally Posted by awsdert

Don't know what you mean here

---

You wrote "as for linked lists there are cases for it such as trees". That's a wrong example.

Quote:

---

Originally Posted by awsdert

A struct is over the top in this case:

---

No, it isn't. It is basic abstraction and encapsulation.

Abstraction is only necessary in big projects like IUP, wxWidgets, Qt, jQuery etc, to do it in small projects is just unnecessary work, especially when it's a learner project where abstraction actually makes things harder to grasp, not everyone is like us, grasping concepts right of the bat.

Edit: Just thought I'd point out that my code above has a potential memory leak, I'll leave finding that as a challenge for learners, experienced programmers probably won't find it hard to find

Quote:

---

Originally Posted by awsdert

Abstraction is only necessary in big projects like IUP, wxWidgets, Qt, jQuery etc, to do it in small projects is just unnecessary work

---

Eh. That's the first time I've heard that coming from an experienced programmer. Aside from trivial throwaway snippets, abstraction is always useful. It reduces duplication, helps document the code, and makes it easier to see its structure.

Quote:

---

Originally Posted by awsdert

especially when it's a learner project where abstraction actually makes things harder to grasp, not everyone is like us, grasping concepts right of the bat.

---

If learners don't learn how to do abstraction with trivial learner projects, when will they ever learn?

By the way, why did you allocate a dynamic array and then treat it as a linked list?

Quote:

---

Originally Posted by laserlight

If learners don't learn how to do abstraction with trivial learner projects, when will they ever learn?

---

When they start expanding on these small projects, for example with the MIT Licensed compiler I'm writing, I started with many variables then grouped the common ones together for their relative purposes to simplify existing code then used it as a reference to restart with a less static result that was still correct as I knew what I was doing at that point, might end up doing the same once I comprehend what way to translate the analyzed code into something my linker function will be able to work with, right now I just getting it to recognize when to change what it is looking for and what it is looking for, I even have in mind a method for something all variants of C have yet to produce, in function functions:

Code:

---

int main() {
  int max(int num, int val) { return (num > val) ? num : val; }
  return (max(100,0)==100) ? 0 : 1;
}

---

Quote:

---

Originally Posted by laserlight

By the way, why did you allocate a dynamic array and then treat it as a linked list?

---

That was for allocation efficiency, as you'll note that only happens when count > 1, if count is always given 1 then the loops are simply skipped, useful if you don't plan on doing a lot of moving around, otherwise can just give it 1 every time to make it behave as though the extra was not included

Quote:

---

Originally Posted by awsdert

When they start expanding on these small projects, for example with the MIT Licensed compiler I'm writing, I started with many variables then grouped the common ones together

---

You're not a learner with little clue about how to do abstraction ;)
'Thomson's Rule for First-Time Telescope Makers: "It is faster to make a four-inch mirror then a six-inch mirror than to make a six-inch mirror."'

Quote:

---

Originally Posted by awsdert

I even have in mind a method for something all variants of C have yet to produce, in function functions:

---

It is common in languages inspired by C though, e.g., C++.

Quote:

---

Originally Posted by awsdert

That was for allocation efficiency, as you'll note that only happens when count > 1, if count is always given 1 then the loops are simply skipped, useful if you don't plan on doing a lot of moving around, otherwise can just give it 1 every time to make it behave as though the extra was not included

---

Ah, I see. Now that you explained it, it looks like the beginnings of a double-ended queue implementation, but my bad as I missed it from your example in main because jt looked more like just a dynamic array with unnecessary linking to be used like a linked list but possibly with the disadvantages of both, but that's just a pessimistic perspective with only one call to ins_address.

wow sorry guys im in the uk so only just seen all this. the code snippet in one of my previous posts was borne out of sheer frustration as i was trying to learn a topic that every video or tutorial i read said something slightly different. and the inability to test my program as it wasn't deallocating the memory (or at least appeared it wasn't). i didn't implement click_heres idea as i didn't understand it as i said in a few posts after it. now you have given an example i can see what he meant. and the advantage is i just pass in the struct instead of a pointer to head and tail or in my case head and current.

i get the strong feeling that i am trying to bite of more than i can chew i clearly still don't understand pointers as clearly demonstrated by me doing p_new_element = &get_address_details(); instead of
*p_new_element = get_address_details();

i choose current to point at the one that i was adding to as that was the one i was pointing at. As current is at the moment always pointing to the last node in the list it could also easily be called tail although in one of the video's that i saw none used it.

once i have understood what ive done with my code so far i can go onto adding a node to the middle of the list deleting a single node etc etc.
coop

Quote:

---

Originally Posted by cooper1200

i choose current to point at the one that i was adding to as that was the one i was pointing at. As current is at the moment always pointing to the last node in the list it could also easily be called tail although in one of the video's that i saw none used it.

---

The thing is though, you might have a function for prepending to the head, and another for appending to the tail. So "current" depends on what you want to do, whereas "tail" would always mean the last node.

the other thing i don't understand is as evidenced in the code posted by others it is obviously possible to have head and tail (current) as single pointers and update them accordingly as i saw on the videos. (one used double pointers but without the (*first) stuff. yet when i tried to do it it didn't update anything.

Quote:

---

Originally Posted by Click_here

I've gone and dug up some of the old singly linked list code that I have laying around :)

To get the most out of your LL, seporate your data type from your list by using a void*

Code:

---

typedef struct SList
{
    int Tag; // type of data
    struct sList *Next;
    void *Object;
    size_t Size;
}
SList;

---

---

am i doing things wrong then by mixing my data types with in the struct as you have an int tag. also what is "object" would that be another struct that in my case would be the address details?

What video is that?

Quote:

---

Originally Posted by cooper1200

wow sorry guys im in the uk so only just seen all this. the code snippet in one of my previous posts was borne out of sheer frustration as i was trying to learn a topic that every video or tutorial i read said something slightly different. and the inability to test my program as it wasn't deallocating the memory (or at least appeared it wasn't). i didn't implement click_heres idea as i didn't understand it as i said in a few posts after it. now you have given an example i can see what he meant. and the advantage is i just pass in the struct instead of a pointer to head and tail or in my case head and current.

i get the strong feeling that i am trying to bite of more than i can chew i clearly still don't understand pointers as clearly demonstrated by me doing p_new_element = &get_address_details(); instead of
*p_new_element = get_address_details();

i choose current to point at the one that i was adding to as that was the one i was pointing at. As current is at the moment always pointing to the last node in the list it could also easily be called tail although in one of the video's that i saw none used it.

once i have understood what ive done with my code so far i can go onto adding a node to the middle of the list deleting a single node etc etc.
coop

---

Think of it the way you would get information in books, when you read a book you don't put the book itself inside your head you record with your eyes to short-term memory and the brain decides what information to move to long-term memory, in

Code:

---

*new_addr = get_addr();

---

example the short-term memory is skipped and all information is recorded in long-term memory

Quote:

---

Originally Posted by laserlight

What video is that?

---

sorry i never actually posted the link to the video just said its at 13 mins aprox

here it is YouTube

in this video he just uses single pointers like you guys have YouTube
coop

Quote:

---

Originally Posted by cooper1200

am i doing things wrong then by mixing my data types with in the struct as you have an int tag. also what is "object" would that be another struct that in my case would be the address details?

---

Click_here is just demonstrating a way to have a reusable linked list library.

Personally, I would practice writing a linked list by having a linked list of ints. Not address details, not void*, just int and a next pointer. This way you focus on the logic of the linked list, not on say, trying to allocate space for a string to be stored in a node. Master that, then you can write a linked list for addresses. Master that, then you can write a reusable linked list library with void*.

Quote:

---

Originally Posted by cooper1200

the other thing i don't understand is as evidenced in the code posted by others it is obviously possible to have head and tail (current) as single pointers and update them accordingly as i saw on the videos. (one used double pointers but without the (*first) stuff. yet when i tried to do it it didn't update anything.

---

Again books can be used to grasp the concept, think of each book as the pointers and then think of them on a shelf, since left is normally treated as where they start I'll use that for this analogy, in this analogy the left-most would be the head pointer to begin with and right-most would be the tail pointer to begin with,
when you move books around you normally take all of the ones that are close together (if you can) which would mean the next to your left hand would be your head book and the next to your right hand would be the tail, you would pick these up and insert them elsewhere, the book that is left of them when you put them, in would no longer have the book that was right of it as the next book and instead have the book that was next to your left hand (head remember) as it's next book and likewise the right book would no longer have that book as it's previous book but the one that is next to your right hand (the tail)

Quote:

---

Originally Posted by cooper1200

sorry i never actually posted the link to the video just said its at 13 mins aprox

here it is YouTube

---

You just saw wrongly. The video creator wrote:

Code:

---

newNode->next = NULL;

---

Not:

Code:

---

*newNode->next = NULL;

---

The idea is to work with newNode rather than head because it is simpler to work with a Node* rather than a Node**, then only at the end:

Code:

---

*head = newNode;

---

is that not what i tried? and why pass in a double pointer when the code you guys have supplied only uses a single pointer as well as the second video.

in the first video he passes in **head
then creates a new_node with malloc
then assigns new_node to *head in the else statement

You have to pass a pointer to a pointer because you want to assign to a pointer in the function such that it is updated in the caller, i.e., you're passing a pointer by reference. You don't have to do this if you return the pointer instead such that the caller can assign the return value to the pointer in the caller. Likewise, you don't have to pass a pointer to a pointer if you're only modifying what the pointer points to, not assigning to the pointer itself.

i see where i am getting confused i just looked at my code..... i do exactly the same but go one step further in my function and set (*first) -> next which he doesn't.

however in the other video it was all done with single pointers

so because i am tying to modify my head in a separate function to where it was declared i have a double pointer??? and have (*first) trust me to make it doubly difficult!!

i think i need to go away and implement your suggestion of having one data type (an int) and start again from basics.

incidently i notice that we always check to make sure malloc has returned a valid pointer ie not null. should we also check that we have actually freed that chunk of memory with the calls to free??

free() doesn't return anything.

It is good practice to assign NULL to a pointer that you just free'd, so if you accidentally try to use it again you'll know.

I agree on just using ints - I think that I was just getting a bit excited :P

free() doesn't return anything, we're supposed to assume it succeeded which it probably does as from what I recall malloc & co can't allocate the small bits of memory we want by themselves, it has to allocate an entire page of memory (just think of it as something the system uses to optimise switching in and out of user space) and then pass a pointer from within that memory, that as assuming it didn't find a page of memory with enough space on it from the start before trying to allocate the memory

Quote:

---

Originally Posted by cooper1200

so because i am tying to modify my head in a separate function to where it was declared i have a double pointer??? and have (*first) trust me to make it doubly difficult!!

---

Yes and yes :)

For example, suppose we have a singly linked list without tail of ints that we denote by just a head pointer to a node struct:

Code:

---

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

---

We could initialise the linked list with a single node with something like this:

Code:

---

void linked_list_init(struct node **head, int data)
{
    *head = malloc(sizeof(**head));
    if (head)
    {
        (*head)->data = data;
        (*head)->next = NULL;
    }
}

---

But at the price of having a local node pointer, we could make it a little easier to read with this:

Code:

---

void linked_list_init(struct node **head, int data)
{
    struct node *node = malloc(sizeof(*node));
    if (node)
    {
        node->data = data;
        node->next = NULL;
    }
    *head = node;
}

---

In both cases, we would call the code like this:

Code:

---

struct node *head = NULL;
linked_list_init(&head, 123);

---

In this case, we didn't need the pointer to a pointer because we could have gone with:

Code:

---

struct node *linked_list_init(int data)
{
    struct node *head = malloc(sizeof(*head));
    if (head)
    {
        head->data = data;
        head->next = NULL;
    }
    return head;
}

// ...

struct node *head = linked_list_init(123);

---

Quote:

---

Originally Posted by cooper1200

i think i need to go away and implement your suggestion of having one data type (an int) and start again from basics.

---

Yes. Try a singly linked list with tail with a struct for the linked list. After you have implemented various operations for a linked list of ints, try a linked list of char* which for a linked list of dynamically allocated strings. Then try a linked list of Address* where Address would be a struct representing an address (i.e., you separate the address details from the node). After that, if you want to try void* as a member of the node for a reusable linked list, it would make more sense to you.

Quote:

---

Originally Posted by cooper1200

incidently i notice that we always check to make sure malloc has returned a valid pointer ie not null. should we also check that we have actually freed that chunk of memory with the calls to free??

---

You cannot check that a call to free() was successful, i.e., you have to assume success or no-op (in the event it was passed a null pointer). Notice that the parameter to free is of type void*, i.e., from within free, the pointer in the caller cannot be set to a null pointer because free only has a copy of that pointer converted to void*.

i have managed to confuse myself already

Code:

---

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

typedef struct Node
{
    int age;
    struct Node *next;
}Node;

typedef struct LinkedList
{
    struct Node *head;
    struct Node *tail;
}LinkedList;

struct LinkedList *create_head(void);

int main()
{
    //declare my list
    LinkedList list;
   
    //set both head and tail pointers to null
    list.head = NULL;
    list.tail = NULL;
   
    list.head = create_head();
   
    return 0;
}

struct LinkedList *create_head(void)
{
    struct Node *head_tmp;
   
    *head_tmp = malloc(sizeof(*head));
   
    if (!head_tmp) //malloc failed
    {
        printf("Memory allocation failed\n")
    }
    return *head_tmp;
}

---

i have created (i think) the head_tmp node. and assigned the pointer to linkedlist.head. but now i need to put data in it and set the tail the same

Eh. head_tmp is the pointer, *head_tmp is what the pointer points to, i.e., a Node object.

its worse than being confused it doesn't compile error line 36 incompatible types when assigning to type struct node from type void

and there is an error in my code as i changed the variable from head to head tmp and forgot to update the call to malloc

and there is an error in my code as i changed the variable from head to head tmp and forgot to update the call to malloc

all i want to do is have the function return a pointer to a chunk of memory that is the size of node and assign it to a pointer called head. just like laserlight did in her code. just like every example i have seen dones

Quote:

---

Originally Posted by cooper1200

all i want to do is have the function return a pointer to a chunk of memory that is the size of node and assign it to a pointer called head. just like laserlight did in her code. just like every example i have seen dones

---

Ok. here's an interesting way to do it with polymorphism... Say you have a list_T type defined as:

Code:

---

// structure to keep track of the list.
typedef struct list_s {
  node_T *head;
  node_T *tail;
} list_T;

---

And your note_T type defined as:

Code:

---

// generic node.
typedef struct node_s {
  struct node_s *next;
} node_T;

---

If all your elements on the list is defined as:

Code:

---

struct element_s {
  node_T *next; // this pointer MUST be here always;

  // your data (3D vetor coordinates as example)...
  double x, y, z; 
};

---

You can deal with your list like this:

Code:

---

// Used to initialize your empty list.
#define EMPTY_LIST { NULL, NULL }

void addhead( list_T *lstp, void *elemp )
{
  node_T *p = elemp;

  p->next = lstp->head;
  lstp->head = p;  // our element will be the new head.

  // if the list is previously empty, we have a new tail.
  if ( ! lstp->tail )
    lstp->tail = p;
}

---

Of course you'll need to allocate your new element somewhere. Example of use:

Code:

---

list_T list = EMPTY_LIST;
struct element_s *elemp;

// allocate element:
elemp = malloc( sizeof *elemp );

// fill the data
elemp->x = 1.0f;
elemp->y = 0.0f;
elemp->z = sqrt(2.0);

// insert element in the list:
addhead( &list, elemp );

---

ok this is getting beyond a joke now

Code:

---

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

typedef struct Node
{
    int age;
    struct Node *next;
}Node;

typedef struct LinkedList
{
    struct Node *head;
    struct Node *tail;
}LinkedList;

struct LinkedList *create_head(void);

int main()
{
    //declare my list
    LinkedList list;
    Node data;

    //set both head and tail pointers to null
    list.head = NULL;
    list.tail = NULL;

    list.head = malloc(sizeof(data));
    if (!list.head)
    {
        printf("error allocating memory\n");
        return -1;
    }
    list.tail = list.head;



    return 0;
}

---

compiles no errors no warnings.

yet all i have done is the same code (literally i cut and pasted it) in a function

Code:

---

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

typedef struct Node
{
    int age;
    struct Node *next;
}Node;

typedef struct LinkedList
{
    struct Node *head;
    struct Node *tail;
}LinkedList;

struct LinkedList *create_head(Node data);

int main()
{
    //declare my list
    LinkedList list;
    Node data;

    //set both head and tail pointers to null
    list.head = NULL;
    list.tail = NULL;

    list.head = create_head(data);

    list.tail = list.head;



    return 0;
}
struct LinkedList *create_head(Node data)
{
    LinkedList *head_tmp;

    head_tmp = malloc(sizeof(data));
    if (!head_tmp)
    {
        printf("error allocating memory\n");
    }

    return *head_tmp;
}

---

produces

Code:

---

linked list simple/main.c|28|warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]|
linked list simple/main.c||In function ‘create_head’:|
linked list simple/main.c|46|error: incompatible types when returning type ‘LinkedList {aka struct LinkedList}’ but ‘struct LinkedList
linked list simple/main.c|47|warning: control reaches end of non-void function [-Wreturn-type]|

---

so the way i read all that is it doesn't see the return statement even though it says the return statement is an error, the error on line 46 is because i passed it a linked list and it wanted a linked list. and s to the warning on line 28 god alone knows
||=== Build failed: 1 error(s), 2 warning(s) (0 minute(s), 0 second(s)) ===|