A simple, buggy app

[geek@02]

Hi, i've made a simple app for fun(with help from the board), which can change user's desktop wallpaper at a given time interval. It's working, but sometimes when i select a picture folder into it, it just hangs up - don't know what's the reason. If u guys could, please checkout what's wrong:

vc++ project
exe file

Thanx.

[karlan]

I reccomend posting any part of the code you think may be causing the problem, or just post all the code if possible

[geek@02]

The code is somewhat long. So i've placed a link to it above -> "vc++ project"

[Salem]

> strcpy(fpath, "/0");
> char sta[MAX_PATH]="\0";
One is a good example of how to empty a string, and the other one isn't.

> char* curti= new char[MAX_PATH];
You never call delete for this, so it's a memory leak?
I can't see why you couldn't use a char array for this bit of temporary memory

> itoa(sti,curti,10);
> _itoa(pos, text, 10);
Lack of consistency here.

> char* val = new char[MAX_PATH];
> val = (char*)QueryRegVal(HKEY_CURRENT_USER,"Software\\Wa lly1\\btnstates","rb_stretch");
More memory leaks here - you don't allocate memory for functions which are returning pointers.

Getting rid of those goto's would be a good move.

Simplify your TimerProc() by putting some code in another function (or two)

You seem to read the registry (eg. "rb_stretch") for every file that matches, when it seems to me like you're going to get a constant answer in effect (do it once before you scan for files?)

> code

// Function to find file extensions
char ext[4]="\0";
char* FindExt(char* path)

Ewww - random global variable just so you can return it.
Ewww - you manage to overwrite your \0 with junk data
What happens if you have a file like "this.is.my.jpg" ?
There is (iirc) a function called splitpath()
Better yet, how about using strrchr() to find the LAST dot in a filename, not a home-grown loop to find the first dot.

> int StartWC(char* fpath, int ti)
All very well returning 0 in the if, but what garbage value are you returning in the else part?
What if that is randomly 0 as well?

> return pVal;
> delete pVal;
I don't think that delete is going to do you any good.

I would suggest you turn up your compiler warning level to maximum. Maybe it would spot such unreachable code.

[geek@02]

Thanx Salem for your consulting.

Getting rid of those goto's would be a good move.

Yes i’ve heard that goto is not good. I’m gonna rethink a way to do without them.

Better yet, how about using strrchr() to find the LAST dot in a filename, not a home-grown loop to find the first dot.

Didn’t knew about strrchr(). tnx.

> int StartWC(char* fpath, int ti)
All very well returning 0 in the if, but what garbage value are you returning in the else part?

What must i do to prevent that warning C4715 here?

> return pVal;
> delete pVal;
I don't think that delete is going to do you any good.

Can u please explain that more?

Thanx.

[Salem]

> What must i do to prevent that warning C4715 here?
Well you click on the warning number, press F1 and start reading.
The words seem pretty obvious to me.

> Can u please explain that more?
Well look at the code - how often do you think the delete pVal; will actually happen when there is a return statement in front of it?

[geek@02]

Ok thanx.
But the problem remains. Sometimes (only sometimes)when i select a picture folder the app just gets stuck. The CPU usage meter hits 100%. It must be something about the folder. But the folder seems fine only with some pics in it. If anybody else detect that bug, please report.
Thanx again.

[novacain]

WOW!

I think you should look at the C/C++ switch / case statements (to replace the endless IFs

You are losing a lot of GDI resources particularly in WM_INIT (the bitmaps have to be freed (DeleteObject() or the memory is lost, not while selected into a HDC however)

Your paint has errors.....
The HDC you create is not DeleteDC()ed nor is the BMP DeleteObject()ed.

What happens if the bmp is bigger than the screen? Where do you start drawing? (there is a rect in the PAINTSTRUCT)

Little error checking (for null handles) and no error handling....

I would..
BROWSEINFO bi;
::ZeroMemory(&bi,sizeof(BROWSEINFO));//use size of struct not size of declared variable ie what happens if it is a pointer

Thats as far as I got....time to learn debugging......