Thread: Linked list pointer assignment

Okay, I'm just trying to insert values into a linked list (structs), and I must be doing the assignments wrong, but I can't see what it might be. Is it not simply tempPtr = tempPtr->next ? Can someone point me in the right direction here?

As usual, thanks in advance!

Code:

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

struct personStruct{
	char *fn, *ln;
	struct personStruct *next;
}*root = NULL;

void display(struct personStruct* ps);

int main(void){
	void insert(struct personStruct *ps, char *fn, char *ln);
	char response[999], firstName[999], lastName[999];
	char *resp = response, *first = firstName, *last = lastName;

	while(1){
		printf("%s", "\nPress (9) to quit; otherwise, any character to insert: ");
		fflush(stdout);
		scanf("%s", resp);
		if(resp[0] == '9'){
			return 0;
		}
		else{
			printf("\nPlease enter the first name: ");
			fflush(stdout);
			scanf("%s", first);
			printf("\nPlease enter the last name: ");
			fflush(stdout);
			scanf("%s", last);
			insert(first, last);
		}
	}
}

void insert(char *first, char *last){
	struct personStruct *tail = malloc(sizeof(struct personStruct));
	tail->fn = first;
	tail->ln = last;
	tail->next = NULL;
	if(root == NULL){
		printf("%s", "\nroot = NULL");
		fflush(stdout);
		root = tail;  //assign new value to root if empty list
	}
	else{
		struct personStruct *temp = root;  //else start a temp pointer at root to traverse the list
		printf("\n%s %s %s %s", "In insert(), root->fn = ", root->fn, ", and root->ln = ", root->ln);
		fflush(stdout);
		while(temp->next != NULL){  //until end of list, display each struct's values
			printf("\n%s %s %s %s", "In insert(), temp->next->fn = ", temp->next->fn, ", and temp->next->ln = ", temp->next->ln);
			fflush(stdout);
			temp = temp->next;  //jump to next struct
		}
		fflush(stdout);
		temp->next = tail;  //end of list reached, put inserted node at end
	}
	display(root);
}

void display(struct personStruct *root){
	struct personStruct *temp = root;
	if(root == NULL){
		printf("%s", "Uh-oh, root = NULL");
		fflush(stdout);
	}
	else
		while(temp != NULL){
			printf("%s %s %s", "In display: ", temp->fn, temp->ln);
			fflush(stdout);
			temp = temp->next;
		}
}

temp = temp->next; is valid. but you are using the same 'first' and 'last' variable for each new list element, so they are overwritten each time you enter a new value. you need to provide space for the first and last in each struct when you insert it.

You have a lot of things you should fix in your program. In no particular order:

• Global variables are bad. Read this link. Declare root in main, and pass it to your insert and display functions.
• Prototypes belong outside functions. They must match the function definition in name, return type, and number, order and types of parameters.
• You should remove as many calls to fflush(stdout) as possible. They're not wrong, but they're extra, unnecessary stuff in your code. Functions that ask for input (getchar, scanf, etc) flush the output automatically, so definintely remove all the fflush calls just before input functions. Also, printing a new line flushes the output, so you can print a new line at the end of a line of output, instead of at the beginning. That is, printf("\n%s %s %s %s", ...) could be printf("%s %s %s %s\n", ...).
• You should avoid while(1) loops wherever possible. Do while loops are great for user input. Example below.
• You should make sure to free up your list when you're done.
• You don't need to make a pointer that points at the beginning of each char array. Just use the name of the array, e.g. scanf("%s", lastName).
• Your struct only has char pointers. Every time you insert, you set fn and ln to point to the parameters, which turn out to just be firstName and lastName from main, so every node in your list points at the same array in main, thus all entries will have the same name. Either make your struct contain char arrays, or malloc enough space for the fn and ln fields (remember, +1 for the null character!). If you malloc space for them, make sure you free that malloc'ed memory before you free the node.

That's all I got in my 5-minute overview, but it should be enough for you to chew on for a while.

Code:

do {
    read user input
    if it's not 9
        get first and last name
        insert
} while (input[0] != '9');

Okay, this makes sense: since pointers contain addresses, I was simply changing the value of what was stored in that address, and since I had fn and ln point to firstName and lastName in each struct, the value referenced changed, making the value of the char pointed to change in each of them. Thanks to both of you, and anduril, I appreciate all of the programming style words of wisdom! I did get a bit sloppy here, but I was just scratching something up really quickly to review the basic data structs. That aside, there were a few things in that list that I wasn't awar of. Thanks again!

> You should remove as many calls to fflush(stdout) as possible.
Right, and if you're having to fflush(stdout) too many times, you should probably be printing to stderr: it's unbuffered output to the same place.

Indeed, the below works. Thanks for the tip, memcpy.

Code:

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

struct personStruct{
	char fn[999], ln[999];
	struct personStruct *next;
}*root = NULL;

void display(struct personStruct* ps);
void insert(char fn[], char ln[]);

int main(void){

	char response[999], firstName[999], lastName[999];
	char *resp = response;

	while(1){
		printf("%s", "\nPress (9) to quit; otherwise, any character to insert: ");
		fflush(stdout);
		scanf("%s", resp);
		if(resp[0] == '9'){
			return 0;
		}
		else{
			printf("\nPlease enter the first name: ");
			fflush(stdout);
			scanf("%s", firstName);
			printf("\nPlease enter the last name: ");
			fflush(stdout);
			scanf("%s", lastName);
			insert(firstName, lastName);
		}
	}
}

void insert(char first[999], char last[999]){
	struct personStruct *tail = malloc(sizeof(struct personStruct));
	strcpy(tail->fn, first);
	strcpy(tail->ln, last);
	tail->next = NULL;
	if(root == NULL){
		printf("%s", "\nroot = NULL");
		fflush(stdout);
		root = tail;
	}
	else{
		struct personStruct *temp = root;
		printf("\n%s %s %s %s", "In insert(), root->fn = ", root->fn, ", and root->ln = ", root->ln);
		fflush(stdout);
		while(temp->next != NULL){
			printf("\n%s %s %s %s", "In insert(), temp->next->fn = ", temp->next->fn, ", and temp->next->ln = ", temp->next->ln);
			fflush(stdout);
			temp = temp->next;
		}
		fflush(stdout);
		temp->next = tail;
	}
	display(root);
}

void display(struct personStruct *root){
	struct personStruct *temp = root;
	if(root == NULL){
		printf("%s", "Uh-oh, root = NULL");
		fflush(stdout);
	}
	else
		while(temp != NULL){
			printf("%s %s %s", "In display: ", temp->fn, temp->ln);
			fflush(stdout);
			temp = temp->next;
		}
}

You're really only supposed to use fflush(stdout) when what you're printing isn't appearing immediately, and you want it to. Forcing it to print to stderr (something you didn't do) will not use buffered i/o, and therefore appear immediately after printing.

You're also not passing by reference. Root is already a pointer, so using the & symbol is unnecessary (and will fail to compile).

Line 15 looks redundant at best, and fatal at worst. Why don't you just scanf() directly into the stack memory allocated in response[], rather than creating a random pointer to it?

Thank you. Any idea why root is still evaluating as NULL here?:

Code:

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



struct personStruct{
	char first[999];
	char last[999];
	struct personStruct *next;
};

int main(void){
	struct personStruct *root = NULL;
	char resp[999];
	do{
		char firstName[999], lastName[999];
		printf("%s", "\nPlease enter a choice, 9 for quit, or any other character to insert: ");
		fflush(stdout);
		scanf("%s", resp);
		if(resp[0] != 9){
			printf("\n%s", "Please enter the first name: ");
			fflush(stdout);
			scanf("%s", firstName);
			printf("\n%s", "Please enter the last name: ");
			fflush(stdout);
			scanf("%s", lastName);
			insert(root, firstName, lastName);
		}
	}while(resp[0] != '9');
	return 0;
}

void insert(struct personStruct *root, char firstName[], char lastName[]){
	struct personStruct *tail = malloc(sizeof(struct personStruct));
	printf("\nFirstname = %s", firstName);
	strcpy(tail->first, firstName);
	strcpy(tail->last, lastName);
	tail->next = NULL;
	if(root == NULL){
		printf("\nRoot = null");
		root = tail;
	}
	else{
		struct personStruct *temp = root;
		while(temp->next != NULL){
			temp = temp->next;
		}
		temp->next = tail;
		free(temp);
	}
	free(tail);
	display(root);
}

void display(struct personStruct *root){
	struct personStruct *temp = root;
	int i = 1;
	while(temp != NULL){
		printf("\n%d: %s %s", i, temp->first, temp->last);
		fflush(stdout);
		temp = temp->next;
		i++;
	}
	free(temp);}

You are passing root by value. Yes, it's a pointer, but you're passing a copy of the address it points to. To change an int from another function, you need to pass a pointer to that int, right? Then to change a pointer to a personStruct, you need to pass a pointer to the pointer:

Code:

int main(void)
{
    struct personStruct *root = NULL;  // root is a pointer, initialize to NULL for empty list
    ...
    insert(&root, firstName, lastName);  // pass a pointer to the pointer
}

// notice insert takes a double pointer
void insert(struct personStruct **root, char firstName[], char lastname[])
{
    ...
    // dereference the double pointer, so it's just a pointer, and we can change root back in main
    *root = some new value;
}

> I was experimenting with omitting fflush(stdout) just before scanf() statements, and no, no text is displayed. Is this compiler-specific, maybe?

By default, stdout is line buffered, meaning it should flush output when the buffer is full or when it gets a new line. Whether it flushes when input is requested turns out to be implementation defined (I just learned something new!) Implementation usually consists of compiler and standard C library, and to some degree OS and architecture (at least insofar as they affect the compiler and library). Most likely it's related to the standard C library you're using, which often comes bundled with a compiler. Out of curiosity, what OS and compiler are you using? Do you know what standard C library you're using?

Actually, looking at limits.h in an attempt to answer a question from another poster, I see that my library is indeed GCC

Actually, by "library", I mean the standard C library. All the standard funcitons, like fgets, scanf, malloc, sqrt, etc are part of a standard library. GCC is the compiler, but it often comes with glibc. That's GNU's implementation of libc, the standard C library. I'm not too familiar with the details of MinGW and how it differs from using GCC on Linux, but it is probably a combination of the version of libc you're using along with how Windows handles stdin and stdout.

As for your umlaut problems, please post your new code. Also, explain what you mean by root never evaluating to NULL. I'm confused, but I think more so you're confused about what level of dereferencing root should produce NULL and when.

First, excuse me; I should have said never evaluates to null . The root is skipped when inserting, and display() causes an infinite loop in which the new value is displayed. Code below:

Code:

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



struct personStruct{
	char first[999];
	char last[999];
	struct personStruct *next;
};

int main(void){
	struct personStruct *root = NULL;
	char resp[999];
	do{
		char firstName[999], lastName[999];
		printf("%s", "\nPlease enter a choice, 9 for quit, or any other character to insert: ");
		fflush(stdout);
		scanf("%s", resp);
		if(resp[0] != 9){
			printf("\n%s", "Please enter the first name: ");
			fflush(stdout);
			scanf("%s", firstName);
			printf("\n%s", "Please enter the last name: ");
			fflush(stdout);
			scanf("%s", lastName);
			insert(&root, firstName, lastName);
		}
	}while(resp[0] != '9');
	return 0;
}

void insert(struct personStruct **root, char firstName[], char lastName[]){
	struct personStruct *tail = malloc(sizeof(struct personStruct));
	printf("\nFirstname = %s", firstName);
	strcpy(tail->first, firstName);
	strcpy(tail->last, lastName);
	tail->next = NULL;
	if(root == NULL){
		printf("\nRoot = null");
		root = tail;
	}
	else{
		struct personStruct *temp = root;
		while(temp->next != NULL){
			temp = temp->next;
		}
		temp->next = tail;
		free(temp);
	}
	free(tail);
	display(root);
}

void display(struct personStruct *root){
	struct personStruct *temp = root;
	int i = 1;
	while(temp != NULL){
		printf("\n%d: %s %s", i, temp->first, temp->last);
		fflush(stdout);
		char c[999];
		scanf(c);
		temp = temp->next;
		i++;
	}
	free(temp);
}

You're not dereferencing root inside insert. Remember, you passed in the address of root from main. Reread post #10 carefully, and see if you can spot your error.