Member Avatar for donaldunca

I have some problem with linked list FIFO(first in first out).
This is my code:

typedef int datatype;
typedef struct node *nodep;
struct node *first,*last,*p;
struct node
{
    datatype data;
    nodep *next;
};
void create();
void list();
void create()
{       char *tl;
    do
    {
        p=(struct node*)calloc(5,sizeof(struct node));
        p->data=random(100);
        p->next=NULL;
        if(first==NULL)   first=p;
        else  last->next=p;
        last=p;
        printf("do you still want to create?(y/n):");scanf("%s",&tl);
        getch();
    }while(*tl!='n');
}
void list()
{
    while(p!=NULL)
    {
        printf("%3d",p->data);
        p=p->next;
    }
    getch();
}
main()
{
    first=NULL;
    create();
    list();
    getch();
}

But my program doesn't run. Can you help me to slove this problem?

  • 4 Contributors
  • 5 Replies
  • 316 Views
  • 1 Day Discussion Span
  • Latest Post Latest Post by mathematician

Dani AI

Generated

A compact, practical diagnosis and fix for the compile/run problems reported by , with pointers back to the thread comments from , and .

The core causes were: a typedef that makes nodep a pointer and then declaring next as a pointer-to-pointer (creating type mismatches), allocating five nodes at once instead of one, reading input into an uninitialised pointer, using non‑portable getch() calls, and traversing from the wrong pointer. The corrected approach below uses a clear struct definition, allocates one node at a time, checks allocation, and uses fgets() for safe input.

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

struct Node { int value; struct Node *next; };

static struct Node *head = NULL, *tail = NULL;

static struct Node *make_node(int v) {
    struct Node *n = malloc(sizeof *n);
    if (!n) { perror("malloc"); exit(EXIT_FAILURE); }
    n->value = v; n->next = NULL;
    return n;
}

static void enqueue(int v) {
    struct Node *n = make_node(v);
    if (!head) head = tail = n;
    else { tail->next = n; tail = n; }
}

static void print_list(void) {
    for (struct Node *cur = head; cur; cur = cur->next)
        printf("%3d", cur->value);
    putchar('\n');
}

int main(void) {
    char buf[8];
    srand((unsigned)time(NULL));
    while (1) {
        enqueue(rand() % 100);
        fputs("Add another (y/n)? ", stdout);
        if (!fgets(buf, sizeof buf, stdin) || buf[0] == 'n') break;
    }
    print_list();
    return 0;
}

Checklist and quick notes: prefer struct Node *next over pointer typedefs that hide pointer semantics (this avoids the node*/node* conversion errors); allocate one node with `malloc(sizeof p)orcalloc(1, ...)and always check the return; initialize head/tail to NULL; traverse from the head pointer when printing; avoidgetch()andscanf("%s",...)into uninitialised memory —fgets()orgetchar()are safer; compile with-Wall -Wextra -std=c99to catch the missingmain` return type and other warnings. This addresses the thread observations from , and while preserving a simple FIFO queue API.

Recommended Answers

All 5 Replies

Member Avatar for Salem

A few instant problems

1. Over reliance on global variables.

2. Your print routine starts at 'p', not first. Your p variable is the last node created.

3. scanf("%s",&tl);
t1 doesn't point at any memory.
At a push, the minimum would be these snippets

char t1[10];
scanf("%s",t1);
while( t1[0] != 'n' );
Member Avatar for mathematician

printf("do you still want to create?(y/n):");scanf("%s",&tl);
getch();


It isn't obvious why you need getch() there if you are using scanf to read from the keyboard. That in itself probably explains why your program appears not to run.

The easiest thing you could do would be to redefine t1 as a character (not a pointer), and then use:

t1 = getch();

or

t1 = getchar();

In fact you seem to have calls to getch() sprinkled throughout your program, without any corresponding prompts being displayed on the screen. The only thing that is likely to do is bring the program to a grinding halt.

Member Avatar for donaldunca

My mistakes of program:
- Can not convert 'node*' to 'node**'
- Can not convert 'node**' to 'node*'
- Function should return a value
I don't understand these mistakes. Could you help me?Thank you!!

Member Avatar for Lazaro Claiborn

start-quote

void create()
{ 
    char *tl;
    do
    {
        p=(struct node*)calloc(5,sizeof(struct node));
        p->data=random(100);
        p->next=NULL;
        if(first==NULL) first=p;
        else last->next=p;
        last=p;
        printf("do you still want to create?(y/n):");scanf("%s",&tl);
        getch();
    }while(*tl!='n');
}

end-quote

calloc will allocate 5 node structs. p will point to the first of the five elements. On the second loop, last will be assigned the new allocate 5 struct node objects, but then last will be assigned p. Second loop; Since last at this point points to the first allocated 5 objects, the "last=p" would overwrite the first allocate 5 objects with the secondly new allocated 5 objects; The "else last->next" really is irrelevant at this point. You might want to use malloc instead of calloc because malloc could be used to allocate one object at a time instead of 5 using calloc. Or you can get the user's input and calloc that number of objects and loop through initializing them. Let me know.

Good luck, LamaBot

Edited by mike_2000_17 because: Fixed formatting
Member Avatar for mathematician

To append a comment to the previous. If you allocate an array of structures by using calloc you do not really need a linked list at all - unless you are, or might be, allocating more structures subsequently. The point of a linked list is link together structures which may not be in a single contiguous block of memory. If they are known to be in a single contiguous block of memory, it is easier to address them as an array.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.