Reputation: 63
I'm trying to implement a linked-list data structure which each node has a identifier key, some data of variable length (malloc), and a pointer to the next node. Now I want to have 3 functions which respectively: sets a new node to the front of the list, prints the values of a given node using identifier key, and deletes a given node.
The struct I have for the node is as follows:
struct node {
char key[5];
int* data;
node* next;
};
struct node* headNode = NULL;
I have questions regarding each of functions. I will list the function codes I have and ask questions regarding that specific function below:
The code for my set function:
void command_set (char key[], int val[], int numOfVal){
struct node* temp = (node*)malloc(sizeof(node));
strcpy(temp->key, key);
temp->data = (int*)malloc(numOfVal*sizeof(int));
*(temp->data) = *(val);
temp->next = entry_head;
entry_head = temp;
return;
}
Now I have one question regarding this function:
1) Is my method of storing the data valid? i.e. "temp->data = (int*)malloc(numOfValuessizeof(int));" + "(temp->data) = *(val);". What I'm trying to do is dynamically allocate some memory, then store the given values as my node's data in that memory.
The code for my print function:
void printNode (char key[], int numOfVal){
int i;
struct node *currentNode = headNode;
while(currentNode->next!=NULL){
if(!strcmp(currentNode->key,key) ){
for(i=0; i<numOfVal; i++){
printf("%d ",*((currentNode->data)+i));
}
return;
}
currentNode = currentNode->next;
}
I have a one question regarding this function:
2) The data of a node is a list of integers, so does my way of printing out each integer actually work? i.e. "*((currentNode->data)+i)". What I'm trying to do is by using pointer arithmetic I print all the ints stored under data.
The code for my delete function:
void deleteNode (char key[]){
struct node *currentNode = headNode;
struct node *prevNode = headNode;
while(currentNode->next!=NULL){
if(!strcmp(currentNode->key,key) ){
prevNode->next = currentNode->next;
free(currentNode->data);
free(currentNode->next);
free(currentNode);
return;
}
prevNode = currentNode;
currentNode = currentNode->next;
}
I have two questions regarding this function:
3) Am I "deleting" the nodes properly? By using free(). Is this the way to do it?
4) Is this how you link up nodes after deletion? By setting the next pointer to another node.
Please assume that malloc will not return NULL for simplicity. Also note that I have simplified my actual code, else there is way too much to post, so there might be slight errors. You may also assum that the while loops will always work (i.e. there will not be a case where (currentNode->next==NULL). The main point of this post are my questions regarding whether the method of doing something is correct.
An example of the program would be:
-set ex1 2 3 4 5
-get ex1
2 3 4 5
-set ab 32 112
-get ab
32 112
Thanks in advance.
Upvotes: 0
Views: 474
Reputation: 543
strcpy(temp->key, key);
For the the purpose of your program, this is probably ok, but you should use strncpy(temp->key,key,5) to be safe. Or at least check the length of key to make sure it fits.
*(temp->data) = *(val);
This only sets the first index in the array. You should use memcpy here.
memcpy (temp->data,val, sizeof (int) * numOfVal);
Your print function prints the first element that doesn't match. Did you mean to do the opposite?
Your delete function does the thing. It finds the first node that doesn't match.
You also don't want to free currentNode->next;
Upvotes: 1