Linked List Help

[tar]

I am having trouble with the code below, for some reason there are no recipients in my linked list. I think it's a pointer problem. Also in my rcpt_add function is it best to return the head of the list? The problem I see with that is telling if the malloc in the function failed for the new node. Any suggestions would be appreciated, thanks.

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

typedef struct rcptnode {
	char data[16];
	struct rcptnode *next;
} rcpt;

rcpt *rcpt_add(rcpt *node, char *value) {
	rcpt *tmp;
	rcpt *new_node;
	new_node  = malloc(sizeof(rcpt));
	if(!new_node) {
		return NULL;
	}
	
	new_node->next = NULL;
	strncpy(new_node->data, value, 15);
	
	if(!node) {
		node = new_node;
	} else {
		tmp = node;
		while(tmp->next) {
			tmp->next = new_node;
		}
	}
	return new_node;
}

void rcpt_list(rcpt *node) {
	if(node) {
		printf("Recipients:\n");
	} else {
		printf("No recipients\n");
	}
	while(node) {
		if(node->data) {
			printf("Data: %s\n", node->data);
		}
		node = node->next;
	}
}

int main() {
	rcpt *tmp = NULL;
	
	rcpt_add(tmp, "test 1");
	rcpt_add(tmp, "test 2");
	rcpt_list(tmp);
	
	return 0;
}

[Salem]

Close.

Try

		while(tmp->next) {
			tmp = tmp->next;
		}
		tmp->next = new_node;

This runs down the list to the end, then appends the new node.

> strncpy(new_node->data, value, 15);
strncpy() won't append a \0 if the source string is longer than 15
And malloc won't initialise memory to all zeros.
You need to add the \0 yourself.
new_node->data[15] = '\0';

> rcpt_add(tmp, "test 1");
You're ignoring the return result, so the list remains empty.

[tar]

Thanks for the pointers, this code now works. My question is what should I do if the malloc were to fail when adding a new node? Free up the list? Return the list without adding the new node? If I return NULL the rest of the list is just hanging out there causing a memory leak, correct?

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

typedef struct rcptnode {
	char data[16];
	struct rcptnode *next;
} rcpt;

rcpt *rcpt_add(rcpt *node, char *value) {
	rcpt *tmp;
	rcpt *new_node;
	new_node  = malloc(sizeof(rcpt));
	if(!new_node) {
		return NULL;
	}
	
	new_node->next = NULL;
	strncpy(new_node->data, value, 15);
	new_node->data[15] = '\0';
	
	if(!node) {
		node = new_node;
	} else {
		tmp = node;
		while(tmp->next) {
			tmp = tmp->next;
		}
		tmp->next = new_node;
	}
	return node;
}

void rcpt_list(rcpt *node) {
	if(node) {
		printf("Recipients:\n");
	} else {
		printf("No recipients\n");
	}
	while(node) {
		if(node->data) {
			printf("Data: %s\n", node->data);
		}
		node = node->next;
	}
}

int main() {
	rcpt *tmp = NULL;
	int i;
	
	tmp = rcpt_add(tmp, "test 1");
	tmp = rcpt_add(tmp, "test 2");
	rcpt_list(tmp);
	
	return 0;
}

[Salem]

Returning the old list perhaps, but then you also need to return some kind of status to indicate that the list didn't grow.

You could return NULL, but then you need something a bit more careful than saying
list = add( list, data );

Either way, you need to keep the current list to prevent the memory leak you pointed out.