I'm trying to learn C and I'm having some trouble creating a list using pointers.

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

typedef struct _tDateTimeNode
{
    struct _tDateTimeNode *next;
} tDateTimeNode;

typedef struct _ApiData
{
    tDateTimeNode *timeNodeList;
} tApiData;

void dateTimeNode_insert(tDateTimeNode **_list)
{
    bool found = false;
    tDateTimeNode *list = (*_list);
    while (list != NULL && !found)
    {
        printf("This code never runs!\n");
        list = list->next;
    }

    if (list == NULL)
    {
        printf("This if is always executed!\n");
        list = (tDateTimeNode *)malloc(sizeof(tDateTimeNode));
    }

    if (*_list == NULL)
    {
        printf("This if is also executed!\n");
        _list = &list;
    }
}

int main()
{
    tApiData data;
    data.timeNodeList = NULL;

    tDateTimeNode **timeNode = &(data.timeNodeList);
    dateTimeNode_insert(timeNode);
    dateTimeNode_insert(timeNode);
    dateTimeNode_insert(timeNode);
    dateTimeNode_insert(timeNode);
}

After compiling the problem with gcc -o t test.c && ./t, I'm getting the following output:

This if is always executed!
This if is also executed!
This if is always executed!
This if is also executed!
This if is always executed!
This if is also executed!
This if is always executed!
This if is also executed!

As you can see, the last two if are always executed. But the while is never run. The while block should be run after the first execution of the function because supposedly I have added a new element. I think I'm not updating the value of _list correctly, but I have already run out of options to try. Any idea how to solve it?

CodePudding user response：

There are many problems in the way you wrote this

void dateTimeNode_insert(tDateTimeNode **_list)
{
    bool found = false;
    tDateTimeNode *list = (*_list);
    while (list != NULL && !found)
    {
        printf("This code never runs!\n");
        list = list->next;
    }

    if (list == NULL)
    {
        printf("This if is always executed!\n");
        list = (tDateTimeNode *)malloc(sizeof(tDateTimeNode));
    }

    if (*_list == NULL)
    {
        printf("This if is also executed!\n");
        _list = &list;
    }
}

Note that found is always false and can be deleted, since !false is always true, never changed

_list is tDateTimeNode** so it is supposed to bring in the address of the head of the list in *_list

When you write

   _list = &list;

The only reference to the list is gone for good. Maybe you intended to write *_list = list; in order to save the new head of the list when inserting the first element

The naming is also confusing: list, _list.

Note that a list is a container. You should not mix the list with the data. Your list today is of DateTimeNode but it is still just a list. In your next program may be that one of books, the one of songs in a playlist, all classic list exercises. If the code is more generic it is trivial to create lists. All sorts of lists

Example: a list and a DateTimeNode

I will post an example using your code, but using what I wrote above

Imagine a list of tickets to control a queue. Each customer on enter gets a ticket with a sequence number and a timestamp in the format yyyy-mm-dd hh:mm:ss. Customer enter at the end of the queue.

Here is a possible ticket

typedef struct
{
    char*    ts;   // time stamp
    unsigned USN;  // sequence number

}   Ticket;

A possible Node for a list of these tickets

typedef struct _Node
{
    Ticket*       tk;
    struct _Node* next;

}   Node;

As expected each node of the list points to a ticket. A list is just a container.

The list itself could be

typedef struct
{
    Node*    head;  // first node if any
    unsigned size;  // actual Node count

}   List;

These way things are easier to understand: A list has nodes, each node points to some data. The data is numbered, so it is easy to test. And have a timestamp. Sure, computers are fast, and the time stamp resolution is seconds, so we would need thousands of nodes to see different time stamps...

a minimal set of functions

List* create();
List* destroy(List*);
int   insert(Ticket*, List*);
int   show(List*, const char*);

show() accepts an optional string for a message and destroy() returns NULL as a courtesy, so we can invalidate the list pointer in the same line we destroy the list.

A test

int main(void)
{
    Ticket ticket;
    List*  one = create();
    for (int i = 1; i <= 10; i  = 1)
    {
        ticket.USN = i;
        ticket.ts  = get_ts();
        insert(&ticket, one);
    };
    show(one, "After 10 tickets inserted...\n");
    one = destroy(one);  // destroys list, invalidate pointer
    return 0;
}

The code creates a list with 10 tickets, show them and deletes the list.

output

After 10 tickets inserted...

   List has 10 elements:

    1  2021-11-15 14:12:06
    2  2021-11-15 14:12:06
    3  2021-11-15 14:12:06
    4  2021-11-15 14:12:06
    5  2021-11-15 14:12:06
    6  2021-11-15 14:12:06
    7  2021-11-15 14:12:06
    8  2021-11-15 14:12:06
    9  2021-11-15 14:12:06
   10  2021-11-15 14:12:06

Example code

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

typedef struct
{
    char*    ts;   // time stamp
    unsigned USN;  // sequence number
} Ticket;

typedef struct _Node
{
    Ticket*       tk;
    struct _Node* next;

} Node;

typedef struct
{
    Node*    head;  // first node if any
    unsigned size;  // actual Node count
} List;

// List stuff
List* create();
List* destroy(List*);
int   insert(Ticket*, List*);
int   show(List*, const char*);

// helpers
char* get_ts();  // get time in format "yyyy-mm-dd hh:mm:ss"

int main(void)
{
    Ticket ticket;
    List*  one = create();
    for (int i = 1; i <= 10; i  = 1)
    {
        ticket.USN = i;
        ticket.ts  = get_ts();
        insert(&ticket, one);
    };
    show(one, "After 10 tickets inserted...\n");
    one = destroy(one);  // destroys list, invalidate pointer
    return 0;
};  // main()



List* create()
{
    List* list = (List*)malloc(sizeof(List));
    if (list == NULL) return NULL;
    list->head = NULL;
    list->size = 0;
    return list;
}

List* destroy(List* list)
{
    if (list == NULL) return NULL;
    Node* tmp = list->head->next;
    Node* p   = list->head;
    for (unsigned i = 0; i < list->size - 1; i  = 1)
    {  // delete ticket and node at p
        free(p->tk);
        free(p);
        p   = tmp;
        tmp = p->next;
    }
    // p now points to last
    free(p->tk);
    free(p);
    free(list);  // the List itself
    return NULL;
}

int insert(Ticket* tk, List* L)
{  // insert tk into list L, at the end
    if (L == NULL) return -1;
    if (tk == NULL) return -2;
    // get a new ticket
    Ticket* nt = (Ticket*)malloc(sizeof(Ticket));
    *nt        = *tk;
    // now creates a new Node for this ticket
    Node* nnd = (Node*)malloc(sizeof(Node));
    nnd->next = NULL;
    nnd->tk   = nt;

    // list is empty?
    if (L->size == 0)
    {
        L->head = nnd;
        L->size = 1;
        return 0;  // ok
    }

    // search List end
    Node* p = L->head;
    while (p->next != NULL) p = p->next;
    p->next = nnd;  // last now points to new
    L->size  = 1;
    return 0;  // ok
}

int show(List* L, const char* msg)
{
    if (msg != NULL) printf("\n%s\n", msg);
    if (L == NULL)
    {
        printf("There is no list: nothing to display");
        return -1;
    }
    printf("   List has %d elements:\n\n", L->size);
    Node* p = L->head;
    for (unsigned i = 0; i < L->size; i  = 1)
    {
        printf("%5u  %s\n", p->tk->USN, p->tk->ts);
        p = p->next;
    }
    printf("\n\n");
    return L->size;
}

char* get_ts()
{  // return string in format: "yyyy-mm-dd hh:mm:ss"
    time_t moment;
    time(&moment);  // get time
    // convert time to struct tm
    struct tm* t_info     = localtime(&moment);
    char*      time_stamp = (char*)malloc(20);
    // format time stamp and return to caller
    strftime(time_stamp, 20, "%Y-%m-%d %H:%M:%S", t_info);
    return time_stamp;
}