C and struct lists
Author
Heh tnx :) You just in time, a few minutes ago I used memory debug functions _CrtDumpMemoryLeaks, and I did actually found a memory leak :). Well after a few moments of code debugging I realised that 8 bytes * 3 wasn't freed, so after a quick search i noticed, that i didn't free the structure I pointed to, instead of this it pointed to other. So this is what should be done :
I actually use temp value to store the current word pointer, then i update word to next value and finally free the temp structure i stored moments ago. It is absolutely clear now :)
Thank's again :)
//frees memory usedvoid FreeWords(){ //temp value used for freeing memory struct WSTRUCT *temp; while(words != NULL) { if(words->word != NULL) free(words->word); temp = words; //we select next item in the list words = words->next; //but we need to free current item free(temp); } //kill it temp = NULL;}I actually use temp value to store the current word pointer, then i update word to next value and finally free the temp structure i stored moments ago. It is absolutely clear now :)
Thank's again :)
Oops, my bad. I guess I have my imagination back! Well, I'm gonna play with him, later y'all!
If you're going to be fiddling with the list a lot, I'd advise not freeing the cells when you remove them from the list. Instead maintain a second list of free cells. Typically, that will greatly reduces the time spent allocating and deleting cells.
Also, don't ever use sprintf. Either use strndup or snprintf, if that's available.
Note that the length of a string is the number of characters, plus 1 for the terminating NUL character. You may have already dealt with that outside the function, but it's probably better to ensure that you have enough room in this one place than to have to remember to add 1 everywhere the function is called.
In addition, (sizeof char) is guaranteed to be 1. So there's no need to check it.
Also, don't ever use sprintf. Either use strndup or snprintf, if that's available.
// Don't ever do this: words->word = (char *) malloc(length); sprintf(words->word, "%s", words); // Use snprintf: words->word = (char *) malloc(length); snprintf(words->word, length, "%s", words); // Or strndup: words->word = strndup(words, length);Note that the length of a string is the number of characters, plus 1 for the terminating NUL character. You may have already dealt with that outside the function, but it's probably better to ensure that you have enough room in this one place than to have to remember to add 1 everywhere the function is called.
In addition, (sizeof char) is guaranteed to be 1. So there's no need to check it.
Author
Quote:
If you're going to be fiddling with the list a lot, I'd advise not freeing the cells when you remove them from the list. Instead maintain a second list of free cells. Typically, that will greatly reduces the time spent allocating and deleting cells.
That's really interesting,but when dealing with string i still have to allocate new string because i don't know what size of next string would be.If I would have a fixed size of struct then it would be really efficient :). Nice advice.
Quote:
Also, don't ever use sprintf. Either use strndup or snprintf, if that's available.
Maybe you can explain why sprintf is so bad ?
Quote:
Note that the length of a string is the number of characters, plus 1 for the terminating NUL character. You may have already dealt with that outside the function, but it's probably better to ensure that you have enough room in this one place than to have to remember to add 1 everywhere the function is called.
Yep i already had a bug once with size of array less than required to store string. But I would think of something , strlen() is not really the thing to play with :)
Quote:
In addition, (sizeof char) is guaranteed to be 1. So there's no need to check it.
Yep i know that too, just working out the style, still :)
Thank's again
Quote:Original post by DMINATORQuote:
Also, don't ever use sprintf. Either use strndup or snprintf, if that's available.
Maybe you can explain why sprintf is so bad ?
AddWord("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", 1);Although the memory you've allocated is only one byte long, sprintf doesn't know that. It'll write the entire word into memory, overwriting whatever's stored after the allocated memory. That will cause your program to misbehave or crash.
If you used strlen() to find the length of the word, rather than relying upon the parameter, then sprintf would be perfectly safe as you can be sure another thread wouldn't modify the string. But if you have some reason why strlen() wouldn't do, don't use sprintf.
Author
Quote:Original post by MayrelQuote:Original post by DMINATORQuote:
Also, don't ever use sprintf. Either use strndup or snprintf, if that's available.
Maybe you can explain why sprintf is so bad ?
*** Source Snippet Removed ***
Although the memory you've allocated is only one byte long, sprintf doesn't know that. It'll write the entire word into memory, overwriting whatever's stored after the allocated memory. That will cause your program to misbehave or crash.
If you used strlen() to find the length of the word, rather than relying upon the parameter, then sprintf would be perfectly safe as you can be sure another thread wouldn't modify the string. But if you have some reason why strlen() wouldn't do, don't use sprintf.
Yep i got your point, tnx for advice.
Quote:Original post by Mayrelthis maybe completly irrelivant to the OP but not on some systems (i think unicode char == 2 bytes, ala java/C#) for the sake of portability (and argument) sizeof(type) is desirable over any hard-coded values.
(sizeof char) is guaranteed to be 1. So there's no need to check it.
a perfect example is sizeof(int) == sizeof(long) brecause long == int on MS compilers, even though it goes beyond reason that long should be larger than int.
with C/C++, you can hardly guarantee things ;)
Author
Yeah maybe you are right, and btw,
I can use
sprintf(words->word,"%.*s",length,word);
And it does write only the number of bytes specified in length, so I think there is no problems afterall to stick with sprintf() ?
I can use
sprintf(words->word,"%.*s",length,word);
And it does write only the number of bytes specified in length, so I think there is no problems afterall to stick with sprintf() ?
Quote:Original post by silvermaceQuote:Original post by Mayrelthis maybe completly irrelivant to the OP but not on some systems (i think unicode char == 2 bytes, ala java/C#) for the sake of portability (and argument) sizeof(type) is desirable over any hard-coded values.
(sizeof char) is guaranteed to be 1. So there's no need to check it.
a perfect example is sizeof(int) == sizeof(long) brecause long == int on MS compilers, even though it goes beyond reason that long should be larger than int.
with C/C++, you can hardly guarantee things ;)
nitpick [smile]
sizeof(char) is always 1 byte
You might be thinking of wchar_t which designates a wide-character or multibyte character.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
