Reputation: 17
I have been trying to read a string of characters into a linked list until enter is pressed. I want to place each character into separate node.
This is my code:
charNode* readString(){
char c;
charNode *head, *cur, *last;
head = NULL;
while(1){
scanf("%c", &c);
if(c == '\n'){
break;
}else{
if(head ==NULL){
head = malloc(sizeof(struct charNode));
head->c = c;
last = head;
last->next = NULL;
}else{
cur = malloc(sizeof(struct charNode));
cur->c = c;
cur->next = NULL;
last->next = cur;
}
}
}
return head;
}
When I press enter during execution the scanf functiond doesn't seem to detect it at all.
Upvotes: 0
Views: 350
Reputation: 84589
There are a couple of notes about your code. First, as indicated in the comments, you should base your read-loop on the return of the read-function itself. If you want to use scanf
, then you could do:
while (scanf("%c", &c) == 1 && c != '\n')
Though if reading a single character from stdin
, I would recommend the function that does that alone, e.g.
while ((c = getchar()) != '\n' && c != EOF)
Next, within your readString()
function, you fail to update the last
pointer to the last node added as indicated by @JohnnyMopp. You would need to add last = cur;
as the last expression under your else
statement.
Additionally, by declaring head
and tail
within the readString()
function and only returning head
you lose the tail
pointer. It is far better to declare a simple wrapper struct containing the head
and tail
pointers (and any other list statistics you like) and then return a pointer to that wrapper struct to preserve all list information from readString()
. For example:
/** linked list node */
typedef struct node_t {
char data;
struct node_t *next;
} node_t;
/** linked list */
typedef struct {
node_t *head, *tail;
} list_t;
Then by allocating and return a pointer to type list_t
, you preserve the head
and tail
node information. When writing your readString()
function, it may help to pass a FILE*
pointer as a parameter so you can read from any open file-stream you like. If you want to read from stdin
, just pass stdin
as the open stream. It adds a lot of flexibility for the source of your string, while adding little in function complexity. Your readString()
function could be:
list_t *readstr (FILE *fp)
{
if (!fp) /* validate stream not NULL */
return NULL;
int c; /* int to read (must be int for EOF) */
list_t *l = malloc (sizeof *l); /* allocate for list */
if (!l) { /* validate list allocation */
perror ("malloc-l");
return NULL;
}
l->head = l->tail = NULL; /* initialize list head/tail ptrs NULL */
if (fp == stdin) /* if reading from stdin */
fputs ("enter string: ", stdout); /* prompt for string */
while ((c = fgetc(fp)) != '\n' && c != EOF) /* read each character */
if (!add (l, c)) { /* add node, validate */
del_list (l); /* on add failure, free all memory */
l = NULL; /* set pointer NULL */
break;
}
return l; /* return pointer to list on success, NULL otherwise */
}
Approaching it in this manner makes the actual filling, use and freeing of list memory a very simple process. A short main()
would reduce to:
int main (void) {
list_t *l = NULL; /* pointer to list */
if ((l = readstr (stdin))) { /* read string into list/validate */
prn (l); /* print all nodes */
del_list (l); /* free all allocated memory */
}
}
A short implementation of the linked-list of char
reading from stdin
could be as follows:
#include <stdio.h>
#include <stdlib.h>
/** linked list node */
typedef struct node_t {
char data;
struct node_t *next;
} node_t;
/** linked list */
typedef struct {
node_t *head, *tail;
} list_t;
/** add node at end of list, update tail to end */
node_t *add (list_t *l, char c)
{
node_t *node = malloc (sizeof *node); /* allocate node */
if (!node) { /* validate allocation */
perror ("malloc-node");
return NULL;
}
node->data = c; /* initialize members values */
node->next = NULL;
if (!l->head) /* if 1st node, node is head/tail */
l->head = l->tail = node;
else { /* otherwise */
l->tail->next = node; /* add at end, update tail pointer */
l->tail = node;
}
return node; /* return new node */
}
/** print all nodes in list */
void prn (list_t *l)
{
if (!l->head) {
puts ("list-empty");
return;
}
for (node_t *n = l->head; n; n = n->next)
printf (" %c", n->data);
putchar ('\n');
}
/** delete all nodes in list */
void del_nodes (list_t *l)
{
node_t *n = l->head;
while (n) {
node_t *victim = n;
n = n->next;
free (victim);
}
}
/** delete list and all nodes in list */
void del_list (list_t *l)
{
del_nodes (l);
free (l);
}
list_t *readstr (FILE *fp)
{
if (!fp) /* validate stream not NULL */
return NULL;
int c; /* int to read (must be int for EOF) */
list_t *l = malloc (sizeof *l); /* allocate for list */
if (!l) { /* validate list allocation */
perror ("malloc-l");
return NULL;
}
l->head = l->tail = NULL; /* initialize list head/tail ptrs NULL */
if (fp == stdin) /* if reading from stdin */
fputs ("enter string: ", stdout); /* prompt for string */
while ((c = fgetc(fp)) != '\n' && c != EOF) /* read each character */
if (!add (l, c)) { /* add node, validate */
del_list (l); /* on add failure, free all memory */
l = NULL; /* set pointer NULL */
break;
}
return l; /* return pointer to list on success, NULL otherwise */
}
int main (void) {
list_t *l = NULL; /* pointer to list */
if ((l = readstr (stdin))) { /* read string into list/validate */
prn (l); /* print all nodes */
del_list (l); /* free all allocated memory */
}
}
(note: by using the list_t
wrapper, you can declare and fill as many lists as you like and you will preserve both the head
and tail
pointers for each list)
Also, the separate del_nodes()
and del_list()
functions allow you to use a list_t
struct with automatic storage duration (e.g. list_t l = { NULL, NULL };
) and then free()
the allocated nodes alone without calling free()
on the list.
Example Use/Output
$ ./bin/llchargetchar
enter string: my dog has fleas
m y d o g h a s f l e a s
Memory Use/Error Check
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind
is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ valgrind ./bin/llchargetchar
==25923== Memcheck, a memory error detector
==25923== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==25923== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==25923== Command: ./bin/llchargetchar
==25923==
enter string: my dog has fleas
m y d o g h a s f l e a s
==25923==
==25923== HEAP SUMMARY:
==25923== in use at exit: 0 bytes in 0 blocks
==25923== total heap usage: 19 allocs, 19 frees, 2,320 bytes allocated
==25923==
==25923== All heap blocks were freed -- no leaks are possible
==25923==
==25923== For counts of detected and suppressed errors, rerun with: -v
==25923== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
Look things over and let me know if you have further questions.
Upvotes: 2