0

I am currently working on an assignment which requires me to create a struct of elements from input data (name, weight, symbol), and then assign each struct to a linked list before printing it out to the screen.

I am finally at a point where it is taking all of my input data, however, when it is printed out to the screen, the string values are all the same?

This is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define NO_OF_ELEMS 3

typedef struct ELEMENT { 
    const char* name;
    double atomic_weight;
    const char* atomic_symbol;
} elem;

typedef struct list {
    elem data;
    struct list *next;
} list;


list* create_list(elem e)
{
    list* head = malloc(sizeof(e));
    head->data = e;
    head->next = NULL;

    return head;
}

list* add_to_list(elem e, list* h)
{
    list* head = create_list(e);
    head->next = h;

    return head;
}

void print_list(list *h, char * title)
{
     printf("Name\t\tAtomic Weight\t\tAtomic Symbol\n");

     while (h != NULL)
     {
        printf("%s\t\t%lf\t\t%s\n", h->data.name, h->data.atomic_weight, h->data.atomic_symbol);
        h = h->next;
     }
}

elem* create_element(const char *name, double weight, const char *symbol)
{
    elem* e = malloc(sizeof(elem));
    
    e->name = name;
    e->atomic_weight = weight;
    e->atomic_symbol =  symbol;

    return e;
}

int main()
{
    list* list_of_elements;
    int counter = 0;
    while( counter < NO_OF_ELEMS)
    {
        char name[16], symbol[3];
        double weight;

        printf("Please Enter Atomic Element's Name: ");
        scanf("%s", name);

        printf("Please Enter Atomic Element's Weight (Floating Point number): ");
        scanf("%lf", &weight);

        printf("Please Enter Atomic Element's Chemical Symbol: ");
        scanf("%s", symbol);
        
        elem* e = create_element(name, weight, symbol);

        if(counter < 1)
        {
            list_of_elements = create_list(*e);
        }
        else
        {
            list_of_elements = add_to_list(*e, list_of_elements);
        }

        counter++;
        printf("%d / %d\n", counter, NO_OF_ELEMS);

    }
    
    print_list(list_of_elements, "Periodic Table Elements");

    return 0;
}

The code compiles fine, with no errors or warnings. However, this is the output I am getting:

me@my-mbp assessments % ./periodic                      
Please Enter Atomic Element's Name: Helium
Please Enter Atomic Element's Weight (Floating Point number): 0.32244
Please Enter Atomic Element's Chemical Symbol: He
1 / 3
Please Enter Atomic Element's Name: Hydrogen
Please Enter Atomic Element's Weight (Floating Point number): 0.4254
Please Enter Atomic Element's Chemical Symbol: H
2 / 3
Please Enter Atomic Element's Name: Oxygen
Please Enter Atomic Element's Weight (Floating Point number): 0.42534
Please Enter Atomic Element's Chemical Symbol: O2
3 / 3
Periodic Table Elements
Name        Atomic Weight       Atomic Symbol
Oxygen      0.425340        O2
Oxygen      0.425400        O2
Oxygen      0.322440        O2

Can anyone suggest why this is happening?

guyver4mk
  • 609
  • 6
  • 11
  • 1
    It doesn't make sense to malloc one item, then pass it by value to a function which calls malloc again only to do a hard copy of the contents. And I don't really see how you build up a linked list either. – Lundin Mar 11 '22 at 12:40
  • I am rather new to C, so I get that the code is not 100% optimised. My heads been hurting getting my head around pointers, referencing, dereferencing and handling strings lol. The linked list of objects is created & added to with the `create_list` & `add_to_list` functions respectively, by passing the current list into the add function to add the current list to the next field, and adding the current data as the head. – guyver4mk Mar 11 '22 at 12:55
  • 1
    You can't copy a string by copying its pointer. You need `strdup` or `malloc` + `strcpy` instead of `e->name = name;` and `e->atomic_symbol = symbol;`. Looking for the best duplicate ... – Adrian Mole Mar 11 '22 at 13:52
  • 1
    ... as it stands, you have undefined behaviour, because all your `name` and `symbol` members will point to the 'same' local variables - and those cease to exist once the `while` loop is exited. – Adrian Mole Mar 11 '22 at 14:01
  • @AdrianMole - Ah, so even though I have defined them within the while loop so they are new variables being passed to the `create` / `add_to` list functions, they are all pointing to the same address, and therefore being overwritten. Not the behaviour I was expecting, but wouldn't be the first time C has knocked me for 6 lol. I just tested with `strdup` in the `create_element` function and it worked :) – guyver4mk Mar 11 '22 at 15:11
  • Just remember to call `free` to release the memory allocated with `strdup`, when you're done with the parent structure. – Adrian Mole Mar 11 '22 at 15:14

0 Answers0