Windows Message Pump eats my Memory!

Perhaps some example code demonstrating the error would help clarify the problem (and identify the solution).

Skizz

Right! Sure thing!

Here is a snippet of the function. The function takes no arguments and returns the number of entries in the array.
filenames is of type char** and is a global at this point.
ffd is of type WIN32_FIND_DATA

if( !(hfile = FindFirstFile("*", &ffd)))
		return 0;
	
	while(FindNextFile(hfile, &ffd))
		i++;
		
	if( !(filenames = (char**)malloc(i)))
		return 0;
	
	
	for(int x=0; x<i; x++)
	{
		if(!(filenames[x] = (char*)malloc(sizeof(
                     MAX_PATH))))
		{
			return 0;
			free(filenames);
		}
	}
		
	if( !(hfile = FindFirstFile("*", &ffd)))
		return 0;
	i=0;
	
	filenames[i] = ffd.cFileName;
	
	while(FindNextFile(hfile, &ffd))
		filenames[++i] = ffd.cFileName;

	FindClose(hfile);
	
	return i;

Here is the code to free the memory:

if(filenames)
{
     for(int x=0; x<filecount; x++)
	 free(filenames[x]);
	 			
     free(filenames);
}

And here is the message pump:

if(PeekMessage(&lpMsg, NULL, 0, 0, PM_REMOVE))					/* begin the message loop */
{
	if(lpMsg.message == WM_QUIT) 
	{
		break;
	}
	else
	{
		TranslateMessage( &lpMsg );
		DispatchMessage( &lpMsg );
	}
}

[edited by - Areson on April 1, 2004 5:31:04 PM]

Here''s a hint, pointers are bigger than one byte. Look closely in that first section of code and see the obvious error. There may be more errors, I stopped looking when I saw that one.

quote:

---

Original post by Areson

	if( !(filenames = (char**)malloc(i)))
		return 0;

[edited by - Areson on April 1, 2004 5:31:04 PM]

---

Your malloc line is wrong.

[edited by - DrEvil on April 1, 2004 5:37:02 PM]

I didn''t read it to carefully, but one thing that is for sure messing things up is
filenames = ffd.cFileName;
You''ll have to use strcpy to copy a string in C, something like
strcpy(filenames[i], ffd.cFileName);

Your use of malloc looks quite strange to...
I would recommend you to read some tutorial and/or book on C programming.

This board rather annoyingly converts [ i] into an < i> tag. Which is pointless because < i> works, and everything else is HTMLized So code often gets whaked...

Hello world! HTML here!

See?

[edited by - RuneLancer on April 1, 2004 6:16:23 PM]

Yeah...I admit it...malloc(i) is pretty stupid. I apologize for not catching that before I posted. The code I posted is not the same as the code I had earlier. The malloc(i) is a result of me playing around with things after they were not working. lol! Sorry for the icky code.

Anyway, something productive did come out of it!
Here's the working code:

if( !(filenames = (char**)malloc(sizeof(char)*i*MAX_PATH)))
		return 0;
	
	
	for(int x=0; x<=i; x++)
	{
		if(!(filenames[x] = (char*)malloc(sizeof(char)* 
                     MAX_PATH)))
		{
			return 0;
			free(filenames);
		}
	}

The second part of the declaration was correct. However, the first part was incorrect. The memory that I needed to allocate was equal to the size of the entire array. This is where the whole thing was falling apart.

Thanks for the help everyone!

[edited by - Areson on April 1, 2004 8:15:45 PM]

[Whince!] Please use STL string and vector or list![Whince!/]

Endurion -- Why? In this particular case, I really don''t need the extra functionalty. I only need an array of strings. Would your reason for suggesting this be to avoid malloc? I''m not sure what you''re getting at so could you explain a little?
Thanks!

Does nobody notice the interesting construction of this form:

<loop>
{
  return 0;
  free(ptr);
} 

free will never get called. Great Big Memory Leak.

quote:

---

Original post by Areson
(Either that, or I''m a comlete programming moron...heh)

---

Strong evidence to support the...

quote:

---

Well, I can safely say that I despise Windows. (Either that, or I''m a comlete programming moron...heh)

---

Windows has a bad reputation, but try to be logical. There''s been tens of thousands of programmers working with the message queue or the FindFirstFile API. If there was a major bug like that with these, you can be sure it would have been found sooner.

You don''t sound like a moron, you sound like a newbie at C. I''d suggest opening a book and trying to understand what a pointer is. It will save you some time in the future.

if( !(filenames = (char**)malloc(sizeof(char)*i*MAX_PATH)))
 

This line still has an error. Mastaba gave you an hint.

Y.

quote:

---

Original post by Areson
Endurion -- Why? In this particular case, I really don''t need the extra functionalty. I only need an array of strings. Would your reason for suggesting this be to avoid malloc? I''m not sure what you''re getting at so could you explain a little?
Thanks!

---

I''m getting at having to work with malloc and take care of a lot of small things, which you needn''t have done. Also you''d be less prone to memory leaks.

I see that it is a pretty good practise though, and gives some deeper insight.

No hard feelings

Lol! Well...I never figured to get this much talk over my question. Oluseyi, I do admit that was odd syntax...however, that was caught and as I did say in an earlier post, the version I tossed you guys was junky...and I apologize.

Ysaneya - Gotcha...I''m allocating waaay too much memory for what I need. Appropriate action will be taken.

Endurion - I see what you mean. Avoiding malloc would be particularily nice, but it''s still good practice at the moment. I''ll remember your suggestion for future reference though!

Thanks a bunch!
Areson

~On the path of learning...but falling wayward from time to time.~

Thanks again to Endurion.

Vectors are a much better (and easier) tool to use instead of dealing with the headache of malloc. I''ve got malloc working the way I want to, but I''m going to use vectors and strings because they are soooo much nicer!
Thanks for the heads up about a powerful tool that I never knew about! (Man...I wish they''d tell us about these things in programming class...I''m gonna chew my profs out...)

Thanks again!
Areson

~On the path of learning...but falling wayward from time to time.~

quote:

---

Original post by Areson
Thanks again to Endurion.

Vectors are a much better (and easier) tool to use instead of dealing with the headache of malloc. I''ve got malloc working the way I want to, but I''m going to use vectors and strings because they are soooo much nicer!
Thanks for the heads up about a powerful tool that I never knew about! (Man...I wish they''d tell us about these things in programming class...I''m gonna chew my profs out...)

Thanks again!
Areson

~On the path of learning...but falling wayward from time to time.~

---

Well, are they teaching you C or C++? If C, then the vector and string classes (or any class, for that matter) don''t exist. Also, you can''t overload the students with too much crap at once. So take it easy on your profs for this (as long as they''re fairly competent, I''ve dealt with profs teaching programming that knew less than me (and I''m not a CSci major))

By the way, if you are using C, write your mallocs like

x = malloc(N * sizeof *x);

this idiom is very nice, and in some ways makes malloc less fragile than C++''s new operator (i.e. it still does the right thing when you change the type of x). And you don''t need the cast in C (it just makes the code more fragile, not more typesafe) (if the compiler complains, tell it you want C and not C++ and it''ll shut up).

if( !(filenames = malloc(i * sizeof *filenames))) {
  return 0;
}

for(int x=0; x<=i; x++) {
  if(!(filenames[x] = malloc(MAX_PATH * sizeof **filenames))) {
    free(filenames);
    return 0;
  }
}