C and struct lists

Started by
21 comments, last by DMINATOR 19 years, 4 months ago
Well I was writing some word list code. It works OK, everything is perfect, but maybe I do miss some memory leaks, it is wery hard to find them .So il ask your help. Can anyone suggest is there a way to improve my code, or are there some mistakes ? i do have some concerns where i pass new struct to temp pointer. tnx in advance.

#include <stdio.h>

struct WSTRUCT;
//this is our words
struct WSTRUCT *words;

void AddWord(char* word,int length);
void FreeWords();

struct WSTRUCT
{
   
   //word
   char* word;
   //pointer to next struct NULL if none
   struct WSTRUCT *next;

};


//frees memory used
void FreeWords()
{
 
	while(words != NULL)
   {
      if(words->word != NULL)
	  free(words->word);

	  words = words->next;
   }
}


void AddWord(char* word,int length)
{

	struct WSTRUCT *temp;
	temp = words;

	//this indicates that the are no 
	//words in a list
	//so we add word and exit
	if(words == NULL)
	{
		//now we create new structure
        words = (struct WSTRUCT*)malloc(sizeof(struct WSTRUCT));

		//we create a new word
        words->word = (char*)malloc(sizeof(char) * length); 
		//and write a value to it
		sprintf(words->word,"%s",word);

		//and point the next pointer
		//to NULL
		words->next = NULL;

		return;
	}
    else

    while(temp->next != NULL)
	{
     temp = temp->next;
	}
	//now temp is the last structure we have
	//so let's make it work

     temp->next = (struct WSTRUCT*)malloc(sizeof(struct WSTRUCT));

		//we create a new word
     temp->next->word = (char*)malloc(sizeof(char) * length); 
		//and write a value to it
     sprintf(temp->next->word,"%s",word);

		//and point the next pointer
		//to NULL
	 temp->next->next = NULL;


	return;
}



int main()
{

  //add new word
  AddWord("word1",6);

  AddWord("word2",6);

  printf("ALLOK\n");

  //frees memory
  FreeWords();
  return 0;
}

Advertisement
The code seems fine to me. Just a few pointers though,
  • When creating a pointer always set it to NULL
  • When freeing a pointer consider setting it to NULL, not really necessary, but can sometimes cause problems
  • Does word really need to be a global


If your really worried about memory leaks have a look at a program called electric fence that can detect memory leaks, in a running program.
I only glanced at your code, the only thing i'll say is you can only make one list of strings with your code.
If you make the destructor of WSTRUCT: delete next, then you only have to delete the first. Remember to set every deleted pointer to NULL or 0.
Quote:Original post by Pipo DeClown
If you make the destructor of WSTRUCT: delete next, then you only have to delete the first. Remember to set every deleted pointer to NULL or 0.


He's using C, no delete operator, no user-defined destructor just plain vanilla functions [smile]
[attention] Thanks for pointing out. [grin] My bad.. XP

However, isn't the C-way to declare structs like "typedef struct {} sStruct;" ??
Quote:Original post by Pipo DeClown
However, isn't the C-way to declare structs like "typedef struct {} sStruct;" ??


You don't have to, its just syntactic sugar for a C programmer [smile]
Great thank's to everyone

Quote:
However, isn't the C-way to declare structs like "typedef struct {} sStruct;" ??


Nice i didn't use it before but, now i read in the book, and it saves me quite a few lines, instead of writing

struct WSTRUCT words = (struct WSTRUCT*)malloc(sizeof(struct WSTRUCT) * 10);

i can write ...

WSTRUCT words = (WSTRUCT*)malloc(sizeof(WSTRUCT) * 10);

it looks and reads far better.


Quote:
The code seems fine to me. Just a few pointers though,

When creating a pointer always set it to NULL

When freeing a pointer consider setting it to NULL, not really necessary, but can sometimes cause problems

Does word really need to be a global



If your really worried about memory leaks have a look at a program called electric fence that can detect memory leaks, in a running program.


1. Where I am missing it ?
2. I am confused here, maybe an example ?
3. Well i only need to use only one variable , so there
is no need to pass it as argument

Thank's for the program advise.


Quote:
I only glanced at your code, the only thing i'll say is you can only make one list of strings with your code.


Yep that's true it's the only number of words lists i should need for now.
Simple really, look at your free function
you release the string, but not the struct that you allocated to hold the string [grin]

[Edited by - Washu on November 25, 2004 2:52:27 PM]

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.

Quote:Original post by Washu
Simple really, look at your free function
you release the string, but the struct that you allocated to hold the string [grin]


Yeah, Washu found it! [lol] You need to release the strings too.. [wink]

Tip: The number of free()'s called should be the same as the number of malloc()'s called.

Lesson to be learned: When you create the string, which you THINK is inside the struct, it is actually somewhere else in the System Memory (RAM). As it is actually a pointer, pointing to a chunk of data. And the struct contains the POINTER and not the chunk of data.

This topic is closed to new replies.

Advertisement