Thread: Major Problems with GetSaveFileName() and GetOpenFileName()

  1. #1
    Self-Taught Programmer CodeHacker's Avatar
    Join Date
    Jul 2004
    Posts
    12

    Exclamation Major Problems with GetSaveFileName() and GetOpenFileName()

    Help! I'm new to this forum, and have been all over the web looking for a solution to the problems in my code. I know there are a lot of requests here, but if you could help with just one of them it would be great! I'm completely self-taught and don't know anyone to ask about this!

    Firstly, whenever GetSaveFileName or GetOpenFileName are called, my programs memory usage goes up by 4MB, and the number of GDI Objects and Handles used more than double. This is not freed when the dialog box closes, and whenever the user clicks OK my program crashes!

    Also my Init function leaks memory, and my paint routines leak a GDI Object unless I delete a bitmap twice!

    Please, if you can help, do! I'm really, really stuck on this one!

  2. #2
    Self-Taught Programmer CodeHacker's Avatar
    Join Date
    Jul 2004
    Posts
    12

    Code for Saving and Loading

    Here s my code for saving and loading. -
    Code:
    switch(wParam)
                {case M_SAVE: if(!Battle){
                    
                    szFileName[0] = '\0'; /* Puts a blank in front of the filename buffer. */
                    ZeroMemory(ofn, sizeof(*ofn)); /* Zeroes out the above class. */
                    ofn->lStructSize = sizeof(*ofn); /* Sets its size. */
                    ofn->hwndOwner = hwnd; /* Sets the window that owns the save file dialog box. */
                    ofn->hInstance = hInstance; /* Sets the application instance that the dialog box will belong to. */
                    ofn->lpstrFilter = "Adventurer Save Files (*.asv)\0*.asv\0All Files (*.*)\0*.*\0"; /* Sets the selectable filters for the dialog box. */
                    ofn->lpstrFile = szFileName; /* Sets the place to store the path of the file to save. */
                    ofn->nMaxFile = MAX_PATH; /* Sets the maximum length of the path. */
                    ofn->Flags = OFN_EXPLORER|OFN_PATHMUSTEXIST|OFN_HIDEREADONLY|OFN_OVERWRITEPROMPT; /* Sets the options for the save dialog box. */
                    ofn->lpstrDefExt = "asv"; /* Sets the default file extension. */
                    
                    if(GetSaveFileName(ofn))/* Gets the filename and path to save to. */
                        
                        {Save_File = CreateFile(szFileName,GENERIC_WRITE|GENERIC_READ,0,NULL,OPEN_ALWAYS,FILE_ATTRIBUTE_NORMAL|FILE_FLAG_SEQUENTIAL_SCAN,NULL); /* Opens or creates the file with write access. */
                        
                        if(Save_File == INVALID_HANDLE_VALUE)
                             {MessageBox(hwnd,"Couldn't Open Save File","--- Error ---",MB_OK|MB_ICONINFORMATION); strcpy(Message,separators[107]+1); Changed = 1; return 1;}
                        
                        SetFilePointer(Save_File,0,NULL,FILE_BEGIN); /* Sets the position in the file to write to. */
                        
                        /* Saves the first inventory item. If successful sends sucess message. */
                        if(WriteFile(Save_File,Inventory1,55,bytes,NULL)){strcpy(Message,separators[108]+1); 
                        
                        //Saves everything else, presuming that if one WriteFile works, so will the rest. */
                        WriteFile(Save_File,Inventory2,55,bytes,NULL);
                        WriteFile(Save_File,Inventory3,55,bytes,NULL);
                        WriteFile(Save_File,Inventory4,55,bytes,NULL);
                        WriteFile(Save_File,Inventory5,55,bytes,NULL);
                        WriteFile(Save_File,Inventory6,55,bytes,NULL);
                        WriteFile(Save_File,Inventory7,55,bytes,NULL);
                        WriteFile(Save_File,Inventory8,55,bytes,NULL);
                        WriteFile(Save_File,Inventory9,55,bytes,NULL);
                        WriteFile(Save_File,Inventory10,55,bytes,NULL);
                        WriteFile(Save_File,InventoryQ,40,bytes,NULL);
                        WriteFile(Save_File,&Exp_To_Next_Level,4,bytes,NULL);
                        WriteFile(Save_File,&Level,4,bytes,NULL);
                        WriteFile(Save_File,&Max_HP,4,bytes,NULL);
                        WriteFile(Save_File,&Strength,4,bytes,NULL);
                        WriteFile(Save_File,&Defence,4,bytes,NULL);
                        WriteFile(Save_File,&HP,4,bytes,NULL);
                        WriteFile(Save_File,&Gold_Coins,4,bytes,NULL);
                        WriteFile(Save_File,Time,8,bytes,NULL);
                        WriteFile(Save_File,&Weapon,55,bytes,NULL);
                        WriteFile(Save_File,&Weapon_Attack,4,bytes,NULL);
                        WriteFile(Save_File,&Armour,55,bytes,NULL);
                        WriteFile(Save_File,&Armour_Defence,4,bytes,NULL);
                        WriteFile(Save_File,&Variables,8,bytes,NULL);
                        WriteFile(Save_File,&Old_Position,4,bytes,NULL);
                        WriteFile(Save_File,&Position,4,bytes,NULL);
                        WriteFile(Save_File,&Sub_Position,4,bytes,NULL);
                        WriteFile(Save_File,&Buying,1,bytes,NULL);
                        WriteFile(Save_File,&Item_To_Buy,4,bytes,NULL);
                        WriteFile(Save_File,&Sell_Price,4,bytes,NULL);
                        WriteFile(Save_File,Position_Name,41,bytes,NULL);
                        WriteFile(Save_File,Position_Text,601,bytes,NULL);
                        WriteFile(Save_File,Position_Commands,501,bytes,NULL);}
                        
                        else strcpy(Message,separators[107]+1); /* If saving failed displays failure message. */
                        CloseHandle(Save_File);} /* Closes save file. */
                        
                        else strcpy(Message,separators[107]+1);} /* If saving failed displays failure message. */
                        else strcpy(Message,separators[109]+1);
                        
                        Changed = 1; return 1;/* Sets program to update. */
                 break;
                 case M_LOAD: if(!Battle){
                    
                    szFileName = '\0'; /* Blanks a char array to hold the path of the file to load. */
                    ZeroMemory(ofn, sizeof(*ofn)); /* Zeroes out the above class. */
                    ofn->lStructSize = sizeof(*ofn); /* Sets its size. */
                    ofn->hwndOwner = hwnd; /* Sets the window that owns the load file dialog box. */
                    ofn->lpstrFilter = "Adventurer Save Files (*.asv)\0*.asv\0All Files (*.*)\0*.*\0"; /* Sets the selectable filters for the dialog box. */
                    ofn->lpstrFile = szFileName; /* Sets the place to store the path of the file to load. */
                    ofn->nMaxFile = MAX_PATH; /* Sets the maximum length of the path. */
                    ofn->Flags = OFN_EXPLORER | OFN_FILEMUSTEXIST | OFN_HIDEREADONLY; /* Sets the options for the load dialog box. */
                    ofn->lpstrDefExt = "asv"; /* Sets the default file extension. */
                    
                    if(GetOpenFileName(ofn)) /* Gets the filename and path to load from. */
                        
                        {Save_File = CreateFile(szFileName,GENERIC_READ,FILE_SHARE_READ,NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL|FILE_FLAG_SEQUENTIAL_SCAN,NULL); /* Opens the file with read access. */
                        
                        if(Save_File == INVALID_HANDLE_VALUE)
                             {MessageBox(hwnd,"Couldn't Open Save File","--- Error ---",MB_OK|MB_ICONINFORMATION); strcpy(Message,separators[111]+1); Changed = 1; return 1;}
                        
                        SetFilePointer(Save_File,0,NULL,FILE_BEGIN); /* Sets the position in the file to write to. */
                        
                        if(ReadFile(Save_File,Inventory1,55,bytes,NULL)){strcpy(Message,separators[112]+1); /* Loads the position. If successful sends success message. */
                        
                        //Saves everything else, presuming that if one ReadFile works, so will the rest. */
                        ReadFile(Save_File,Inventory2,55,bytes,NULL);
                        ReadFile(Save_File,Inventory3,55,bytes,NULL);
                        ReadFile(Save_File,Inventory4,55,bytes,NULL);
                        ReadFile(Save_File,Inventory5,55,bytes,NULL);
                        ReadFile(Save_File,Inventory6,55,bytes,NULL);
                        ReadFile(Save_File,Inventory7,55,bytes,NULL);
                        ReadFile(Save_File,Inventory8,55,bytes,NULL);
                        ReadFile(Save_File,Inventory9,55,bytes,NULL);
                        ReadFile(Save_File,Inventory10,55,bytes,NULL);
                        ReadFile(Save_File,InventoryQ,40,bytes,NULL);
                        ReadFile(Save_File,&Exp_To_Next_Level,4,bytes,NULL);
                        ReadFile(Save_File,&Level,4,bytes,NULL);
                        ReadFile(Save_File,&Max_HP,4,bytes,NULL);
                        ReadFile(Save_File,&Strength,4,bytes,NULL);
                        ReadFile(Save_File,&Defence,4,bytes,NULL);
                        ReadFile(Save_File,&HP,4,bytes,NULL);
                        ReadFile(Save_File,&Gold_Coins,4,bytes,NULL);
                        ReadFile(Save_File,Time,8,bytes,NULL);
                        ReadFile(Save_File,&Weapon,55,bytes,NULL);
                        ReadFile(Save_File,&Weapon_Attack,4,bytes,NULL);
                        ReadFile(Save_File,&Armour,55,bytes,NULL);
                        ReadFile(Save_File,&Armour_Defence,4,bytes,NULL);
                        ReadFile(Save_File,&Variables,8,bytes,NULL);
                        ReadFile(Save_File,&Old_Position,4,bytes,NULL);
                        ReadFile(Save_File,&Position,4,bytes,NULL);
                        ReadFile(Save_File,&Sub_Position,4,bytes,NULL);
                        ReadFile(Save_File,&Buying,1,bytes,NULL);
                        ReadFile(Save_File,&Item_To_Buy,4,bytes,NULL);
                        ReadFile(Save_File,&Sell_Price,4,bytes,NULL);
                        ReadFile(Save_File,Position_Name,41,bytes,NULL);
                        ReadFile(Save_File,Position_Text,601,bytes,NULL);
                        ReadFile(Save_File,Position_Commands,501,bytes,NULL);}
                        
                        else strcpy(Message,separators[111]+1); /* If loading failed displays failure message. */
                        CloseHandle(Save_File);} /* Closes save file. */
                        
                        else strcpy(Message,separators[111]+1);} /* If loading failed displays failure message. */
                        else strcpy(Message,separators[113]+1);
                        
                        Changed = 1; return 1;/* Sets program to update. */
                 return 1;
    Last edited by CodeHacker; 07-08-2004 at 06:54 AM. Reason: Updating Code

  3. #3
    Self-Taught Programmer CodeHacker's Avatar
    Join Date
    Jul 2004
    Posts
    12
    Thanks, anonytmouse.

  4. #4
    Yes, my avatar is stolen anonytmouse's Avatar
    Join Date
    Dec 2002
    Posts
    2,544
    Starting with GetSaveFileName problem.

    Code:
                    szFileName = ""; /* Blanks the filename buffer. */
    No, it doesn't. It sets szFileName to point at the read-only string literal "". When GetSaveFileName() tries to write to this read-only string literal the program will crash. The correct way to clear a string is to put a '\0' character at the beginning:
    Code:
    szFileName[0] = '\0';
    --
    Code:
     Save_File = CreateFile(...);
    You probably want to check if CreateFile succeeded:
    Code:
    if (Save_File == INVALID_HANDLE_VALUE)
    {
       // error - do something
    }
    --
    Code:
                       WriteFile(Save_File,Inventory2,55,bytes,NULL);
    While you have not posted how bytes is declared, I am suspicious. WriteFile expects the address of a DWORD where it can place the number of bytes written. The typical usage is
    Code:
                       DWORD dwWritten;
                       WriteFile(Save_File,Inventory2,55, &dwWritten, NULL);
    If bytes is an uninitialized pointer your program will crash:
    Code:
                       LPDWORD bytes;
                       // This will crash as bytes points at random memory.
                       WriteFile(Save_File,Inventory2,55, bytes, NULL);
    --
    In general your code could do with some more space. Your WM_PAINT and init functions are better in this regard. Instead of writing out all the variables seperately, you may consider putting them in a structure so they can be written out in one hit.
    --
    Consider using stack based variables where possible rather than dynamic memory:
    Code:
                    OPENFILENAME ofn = { 0 };
                    TCHAR szFileName[MAX_PATH];
    
                    ofn.lStructSize  = sizeof(*ofn);
                    ofn.hwndOwner    = hwnd;
                    ofn.hInstance    = hInstance;
                    ofn.lpstrFilter  = TEXT("Adventurer Save Files (*.asv)\0*.asv\0All Files (*.*)\0*.*\0");
                    ofn.lpstrFile    = szFileName;
                    ofn.nMaxFile     = MAX_PATH;
                    ofn.Flags        = OFN_EXPLORER | OFN_PATHMUSTEXIST | OFN_HIDEREADONLY | OFN_OVERWRITEPROMPT;
                    ofn.lpstrDefExt  = TEXT("asv");
    --
    You really need three threads here. Could you possibly post the Init and WM_POST problems in different threads? It is a little hard to follow three issues in the one thread.

  5. #5
    Self-Taught Programmer CodeHacker's Avatar
    Join Date
    Jul 2004
    Posts
    12

    Thanks a lot, anonytmouse. But I still can't make it work.

    Thanks a lot for the help! My program's saves are working again!

    I've changed my code round a lot, and it doesn't crash anymore! Unfortunately, it now cannot load any files outside of a PlaySound() command after saving, which (I have opened the resulting file in notepad) now works fine. It's not such an important issue, but I don't understand why my Init and MCIWnd functions can't load after, but the saving functions can.

    P.S - Ive moved the other two problems onto other threads, and updated the saving code I posted first.

    P.P.S - "Bytes" is a pointer to a DWORD created and initialised here, before the program starts -
    Code:
    // Variables for saving and strings.
    HANDLE Save_File;        /* Handle for the save file */
    OPENFILENAME* ofn = new OPENFILENAME; /* Creates a class for the save file dialog box. */
    char* szFileName = new char[MAX_PATH]; /* Creates an array to store the filename for saving. */
    DWORD* bytes = new DWORD;    /* Stores bytes written or read. */                
    
    HANDLE Text_File;        /* Handle for the text files */
    and deleted here, in my response to the WM_DESTROY message -
    Code:
    case WM_DESTROY:
                //Delete dynamically allocated variables.
                
                //Deletes varibles for loading and saving files.
                delete ofn;
                delete bytes;
                
                //Deletes textboxes.
                delete TB_Title;    
                delete TB_Status;
    Just so you know, the textboxes are a class I created to make drawing textboxes on the screen easier.

    Update: My program can save multiple times, but loading never works - the handle is invalid (Thanks for the tip on checking,anonytmouse). Attempting to save after trying to load results in a program crash. Thanks for all those tips anonytmouse. I did have a go at reformating my code.
    Last edited by CodeHacker; 07-08-2004 at 07:01 AM. Reason: Update.

  6. #6
    Yes, my avatar is stolen anonytmouse's Avatar
    Join Date
    Dec 2002
    Posts
    2,544
    Code:
                case M_LOAD: if(!Battle){
                    
                    szFileName = '\0';
    This time, you are setting szFileName to 0!
    --
    Using dynamic memory(allocated with new) everywhere is going to make it near impossible to avoid memory leaks.
    --
    >> P.S - Ive moved the other two problems onto other threads, and updated the saving code I posted first. <<

    I couldn't find them.

  7. #7
    Self-Taught Programmer CodeHacker's Avatar
    Join Date
    Jul 2004
    Posts
    12

    Thanks again, anonytmouse

    Thanks - I didn't spot that bug in my code! Next time I start a program, I won't use dynamic allocation unless I have to. I'm putting the other threads up now.

    The loading works like saving now, so thanks a lot!

    Unfortuately my other handle, HANDLE Text_File; and my MCIWnd still won't load files after saving or loading, so resetting the Init after saving or loading won't work. For the Init I could make it share a handle, but the bug might change to effect other variables.

    I have attached my source files, headers, and resource file to this post all with the extension .txt. Here is a list of the files and their true names. Oh sorry, got to go. I'l add the files from home. Thanks for all you've done! Real World interfering again!
    Last edited by CodeHacker; 07-08-2004 at 08:17 AM.

  8. #8
    Self-Taught Programmer CodeHacker's Avatar
    Join Date
    Jul 2004
    Posts
    12

    I'm Back with my source code!

    Hi! I'm back, and since I'm at home I have Winzip, so I'll include one zip called Adventurer (WIn).zip. It contains all my programs files. If someone has any idea what's up with my program, please will they help - thanks a lot to anonytmouse, who's already done so much to help me.

    The zip doesn't have any viruses - I've got Norton Antivirus 2004 fully up to date with it's definitions running here.
    Last edited by CodeHacker; 07-08-2004 at 12:48 PM. Reason: Posting Zip Correctly

  9. #9
    Self-Taught Programmer CodeHacker's Avatar
    Join Date
    Jul 2004
    Posts
    12

    Zip Update

    This is an updated version of my zip as edited in response to advice from many good people on two other threads.
    --- I think I think, therefore I might be ---

    Why call myself the Code Hacker when I'm only a beginner? Because every day I log onto my PC and hack 'n' slash my way through line after line of my mixed up code, trying to make it work. And failing miserably.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Troubles with GetOpenFileName()
    By lyx in forum Windows Programming
    Replies: 13
    Last Post: 10-28-2003, 09:47 PM