Creating linked list of strings

Question

I am new to data structures as well as to linked lists. I am working on project named as Amazon product availability checker using tree in C. So I want to store strings in each node of the tree but while storing strings the code is not showing any error but the output is also not getting printed. I have pass the node to print function to print the string but nothing is getting printed.

I have shared the code for one string and one node only. I am working on ubuntu and I am coding in the C language.

Here is my code:

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

typedef struct node {
    char clothing[10];
    struct node *next;
} node;

// 1) creating node

node *create(char const ch[]) {
    int i;
    node *head = NULL;
    node *temp = NULL;
    node *p = NULL;
    temp = (struct node *)malloc(sizeof(struct node));
    temp->clothing[10] = ch[10];
    temp->next = NULL;
    return temp;
}

// 2) print 

void print(node *head) {
    node *p = head;
    while (p != NULL) {
        printf("%s", p->clothing);
        p = p->next;
    }
}

int main() {
    node *head = create("clothing");
    print(head);
}

The most appreciated way of showing code here is a minimal reproducible example. You present more of a code puzzle. Also, it is not clear what you are asking. — Yunnosch
– Yunnosch, Commented Apr 6, 2019 at 15:39
Please learn about indentation and comments. Both would help to make your code more readable and less fractured. — Yunnosch
– Yunnosch, Commented Apr 6, 2019 at 15:41
In create, temp and p are unused, so delete them. And use strcpy to copy the string. temp -> clothing[10] = ch[10] is assigning a single character, and in fact that character is out of bounds so you are corrupting memory. — Tom Karzes
– Tom Karzes, Commented Apr 6, 2019 at 15:43
@TomKarzes can you just elaborate your answer. It will mean a lot. Thank you. — ALMAS SANGLE
– ALMAS SANGLE, Commented Apr 6, 2019 at 15:46
@chqrlie Yeah sure....I was not knowing how to accept answer on stack overflow....thanks!! — ALMAS SANGLE
– ALMAS SANGLE, Commented Apr 13, 2019 at 15:17

chqrlie · Accepted Answer · 2019-04-06 15:58:51Z

1

Your create function is incorrect:

you do not test for potential malloc failure
you do not copy the string, but merely cause undefined behavior by attempting to write to clothing[10] which is beyond the end of the array. By the way, you read ch[10] which may well be out of bounds too. You should instead copy the string, while avoiding a buffer overflow if ch is too long.

Here is an improved version:

#incude <string.h>
#incude <stdlib.h>

node *create(char const ch[]) {
    temp = malloc(sizeof(node));
    if (temp != NULL) {
        temp->next = NULL;
        temp->clothing[0] = '\0';
        strncat(temp->clothing, ch, sizeof(temp->clothing) - 1);
    }
    return temp;
}

Since C99, there is a way to allocate a copy of the string without a limitation on its size, and without the need for a separate allocation and a pointer in the node structure. It is called a flexible array. Here is how it works:

typedef struct node {
    struct node *next;
    char clothing[];
} node;

node *create(char const ch[]) {
    size_t size = strlen(ch) + 1;
    temp = malloc(sizeof(node) + size);
    if (temp != NULL) {
        temp->next = NULL;
        memcpy(temp->clothing, ch, size);
    }
    return temp;
}

edited Apr 6, 2019 at 15:58

answered Apr 6, 2019 at 15:52

chqrlie

152k12 gold badges145 silver badges231 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

ALMAS SANGLE Over a year ago

@chqrlie Thank you so much!! It worked very well. It means a lot.

0___________ · Accepted Answer · 2019-04-06 16:12:04Z

0

node *addnode(node *after, const char *str)
{
    node *nptr;

    nptr = malloc(sizeof(*nptr));
    nptr -> partstr = malloc(strlen(str) + 1);
    /* error checking you need to add after every malloc */
    strcpy(nptr -> partstr, str);
    if(!after)
    {
        nptr -> prev = NULL;
        nptr -> next = NULL;
    }
    else
    {
        after -> next -> prev = nptr;
        nptr -> next = after -> next;
        after -> next = nptr;
        nptr -> prev = after;
    }
    return nptr;
}

edited Apr 6, 2019 at 16:12

answered Apr 6, 2019 at 15:52

0___________

71.6k4 gold badges41 silver badges94 bronze badges

2 Comments

bruno Over a year ago

your function doesn't correspond to the struct node of the OP, there is no char*partstr; nor prev link

0___________ Over a year ago

But is better :)

bruno · Accepted Answer · 2019-04-06 16:18:54Z

0

I have pass the node to print function to print the string but nothing is getting printed.

doing

temp -> clothing[10] = ch[10];

you write (and may be read) one character out of the string, the max index in temp -> clothing is 9

you want something like

strcpy(temp -> clothing, ch);

but take care to not go outside the field clothing because ch is too long

so can be

strncpy(temp -> clothing, ch, sizeof(temp -> clothing) - 1);
temp -> clothing[sizeof(temp -> clothing) - 1] = 0; /* useful if strlen(ch) >= 10 */

are you sure you do not want to replace char clothing[10]; by char * clothing; to not have that limit to 10 ?

edited Apr 6, 2019 at 16:18

answered Apr 6, 2019 at 15:43

bruno

34.3k8 gold badges31 silver badges48 bronze badges

6 Comments

chqrlie Over a year ago

Please do not advocate the use of strncpy, this function is widely misunderstood and causes countless bugs in production code. BTW, you do not need the test in your code, just force the terminator. See this: randomascii.wordpress.com/2013/04/03/stop-using-strncpy-already

bruno Over a year ago

when using it properly it works as expected ;-) But I encourage the OP to not use a constant array size but a char* ^^ yes the test to add the term is useless

chqrlie Over a year ago

Maybe, but most programmers I have met, in person or online, do not use it properly. Its behavior is counter-intuitive, not to mention inefficient. Did you know it pads the destination array with nulls to the size passed as the third argument?

bruno Over a year ago

@chqrlie to pad can be useful, and indicates is not the right way if the constant size is large but again a char * is better. Personaly I think I never used strncpy for me :-)

chqrlie Over a year ago

The padding is almost always useless, as is the case in this particular example. This function was first used in the early Unix system to copy login information to a kernel structure. The login names were limited to 6 letters then, and space was at a premium, so omitting the null terminator was a way to save memory. It should never have made it to the C Standard, but was part of the original C library and people used it, mostly incorrectly from day one.

|

Collectives™ on Stack Overflow

Creating linked list of strings

3 Answers 3

1 Comment

2 Comments

6 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

2 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related