Thread: Working app crashes on other computers

  1. #1
    Registered User
    Join Date
    May 2007
    Posts
    77

    Working app crashes on other computers

    OK, so here's my problem now. This whole bit of code works perfectly (as long as you're converting a certain type of file). However, when I package the whole thing and try to run it on any other computer, it stops responding. I have no clue why that would be. Any help?
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <limits.h>
    #include <stdint.h>
    #include <string.h>
    
    main(int argc, char *argv[])
    {
        FILE* originalSave;
        FILE* newSave;
        long fileSize;
        long finalSize;
        uint32_t *buffer, result = 0;
        int x;
        char *originalSaveName = argv[1],
             *newSaveName = malloc(MAX_PATH),
             *chr;
    
        originalSave = fopen(originalSaveName, "rb");
        
        chr = strrchr(argv[0],'\\');
        strncpy(newSaveName, (char*)argv[0], chr-argv[0]+1);
        
        chr = strrchr(argv[1],'\\');
        strcat(newSaveName, "Wii64 Saves");
        strcat(newSaveName, chr);
        
        newSave = fopen(newSaveName, "wb");
    
        fseek(originalSave, 0, SEEK_END);
        fileSize = ftell(originalSave);
        rewind(originalSave);
        
        if (fileSize < 16384)
            finalSize = 16384;
        else if (fileSize < 32768)
            finalSize = 32768;
        else if (fileSize < 65536)
            finalSize = 65536;
        else if (fileSize < 131072)
            finalSize = 131072;
        else if (fileSize < 262144)
            finalSize = 262144;
        else if (fileSize < 524288)
            finalSize = 524288;
        else if (fileSize > 524288){
            printf("Error:  invalid save size: %d", fileSize/1024);
            return 1;}
    
        buffer = (uint32_t*) malloc(sizeof(char)*fileSize);
        
        for (x = 0; x < fileSize/4; x++)
        {
            fread(buffer, 1, 4, originalSave);
            result = ((*buffer << 24) & 0xFF000000) | 
            ((*buffer << 8) & 0x00FF0000) |
            ((*buffer >> 8) & 0x0000FF00) |
            ((*buffer >> 24) & 0x000000FF);
            fwrite(&result, 1, 4, newSave);
        }
        
        result = 11111111;
        
        for (x = 0; x < (finalSize-fileSize); x++)
        {
            fwrite(&result, 1, 1, newSave);
        }
    
        fclose(newSave);
        fclose(originalSave);
        free(buffer);
    
        return 0;
    }

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Define "package" and "stops responding". If you are copying an executable created with Visual Studio, for example, then you are SOL on another machine unless you install the VS runtime on that other machine.

  3. #3
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    Not sure, but after a quick look, I did notice a few potential problems:

    >> char *originalSaveName = argv[1],

    And what if the program is invoked without arguments? The program ungracefully crashes.

    >> *newSaveName = malloc(MAX_PATH),

    That's not very robust. You should malloc strlen( argv[0] ) + strlen( "Wii64 Saves" ) + 1, just to be sure.

    >> chr = strrchr(argv[0],'\\');

    In the event that the function returns NULL, the next call will crash. Also, just to make sure we're on the same page, argv[0] contains the program name (just seemed odd that it was being used in the way that you are, that's all).

    >> chr = strrchr(argv[1],'\\');

    Again, check your return values, for posterity's sake, at least.

    >> buffer = (uint32_t*) malloc(sizeof(char)*fileSize);

    FYI, the cast is not required in C. And anyway, why are you malloc'ing all that memory, anyway - you only use the first 4 bytes in your loop later on (you might as well do all the read writing with 'result' in that case)?

    >> for (x = 0; x < fileSize/4; x++)

    Even though you have a typedef for uint32, it's best to avoid magic numbers. Prefer sizeof(uint32_t), instead. It would also be a very good idea to do a runtime assertion check on the size to guarantee that it is, in fact, a 32-bit number on a given platform.

    >> fread(buffer, 1, 4, originalSave);

    You should *always* check return values of file-processing functions.

    >> free(buffer);

    Don't forget to free 'newSaveName', as well.

  4. #4
    Registered User
    Join Date
    May 2007
    Posts
    77
    @tabstop: I'm not compiling or building or coding in VS. I really don't like it. Right now, for lack of a better editor/compiler, I'm using Dev-C++.

    @Sebatiani:[QUOTE=Sebastiani;902169]Not sure, but after a quick look, I did notice a few potential problems:

    >> char *originalSaveName = argv[1],

    And what if the program is invoked without arguments? The program ungracefully crashes.[/qoute]It's not supposed to be invoked without arguments. The whole point is to drag and drop onto it. I guess, though, since that is the purpose, I should put in some check to make sure there is an argv[1] to begin with, and exit if there isn't.
    >> *newSaveName = malloc(MAX_PATH),

    That's not very robust. You should malloc strlen( argv[0] ) + strlen( "Wii64 Saves" ) + 1, just to be sure.
    That one I don't have any problems with, lol.
    >> chr = strrchr(argv[0],'\\');

    In the event that the function returns NULL, the next call will crash. Also, just to make sure we're on the same page, argv[0] contains the program name (just seemed odd that it was being used in the way that you are, that's all).
    Umm, when I checked the output of argv[0], it gave me the full path. Is that not the same on every windows platform (at least xp and above)? If not, is there a command I can call to find out the working directory of the application? And yes, I want to do it first with that one because the new file is being created in a subdirectory from the path of the application.
    >> chr = strrchr(argv[1],'\\');

    Again, check your return values, for posterity's sake, at least.
    Alright, I guess I'll stop being lazy. All that means is checking to make sure it's not null, right?
    >> buffer = (uint32_t*) malloc(sizeof(char)*fileSize);

    FYI, the cast is not required in C. And anyway, why are you malloc'ing all that memory, anyway - you only use the first 4 bytes in your loop later on (you might as well do all the read writing with 'result' in that case)?
    Ahh, I had forgotten I had it malloc'ing that much. I was going to try to do the reading a different way, but I figured out this way would be easier. That's a good one to check.
    >> for (x = 0; x < fileSize/4; x++)

    Even though you have a typedef for uint32, it's best to avoid magic numbers. Prefer sizeof(uint32_t), instead. It would also be a very good idea to do a runtime assertion check on the size to guarantee that it is, in fact, a 32-bit number on a given platform.
    Huh? Are you saying I should use uint32_t for the type for fileSize? I'm not using the total possible size of uint32_t for fileSize, fileSize has its own value that is critical to function of that particular if..else if... sequence. What you're saying for that doesn't make sense in the context of my application.[quote]>> fread(buffer, 1, 4, originalSave);

    You should *always* check return values of file-processing functions.[quote]Alright, another laziness issue. It's worked so far, though, because it's a very specific type of file it's handling.
    >> free(buffer);

    Don't forget to free 'newSaveName', as well.
    Gotcha. I always forget to add those in.

  5. #5
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Quote Originally Posted by Molokai View Post
    However, when I package the whole thing and try to run it on any other computer, it stops responding. I have no clue why that would be.
    Probably because of memory segmentation. When a compiler puts an executable together, it has a little freedom in terms of what gets stored exactly where. So different compilers (even just different version numbers) and particularly different architectures (eg, 32-bit vs. 64-bit) will produce slightly different results.

    Remember, a bug is not something that is guaranteed to happen. A bug is something that might happen. So if you make a mistake -- such as a buffer overflow -- and nothing bad happens, you don't notice. Rearrange the executable a bit and then bang -- the problem becomes apparent.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  6. #6
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by Molokai View Post
    @tabstop: I'm not compiling or building or coding in VS. I really don't like it. Right now, for lack of a better editor/compiler, I'm using Dev-C++.
    Fair enough. That shouldn't be an issue, then. (Although stops responding means something a little different to me than it seems to for you and Sebastiani -- I took it to mean "I'm waiting for something to happen" while the consensus seems to be "the program exited".)
    Quote Originally Posted by Molokai View Post
    Umm, when I checked the output of argv[0], it gave me the full path. Is that not the same on every windows platform (at least xp and above)? If not, is there a command I can call to find out the working directory of the application?
    argv[0] is how the program was run. You type "programname", you get "programname". You type "programname.exe", you get "programname.exe". If you get a full path by double-clicking then everybody's happy. To get the current working directory you use _getcwd.

    Quote Originally Posted by Molokai View Post
    Huh? Are you saying I should use uint32_t for the type for fileSize? I'm not using the total possible size of uint32_t for fileSize, fileSize has its own value that is critical to function of that particular if..else if... sequence. What you're saying for that doesn't make sense in the context of my application.
    "Magic number" is the number that's written as a number, which in your case was 4. Where did the 4 come from? (Hint: It came from sizeof(uint32_t).)

  7. #7
    Registered User
    Join Date
    May 2007
    Posts
    77
    @tabstop: Yeah, by "package" I mean put the executable together with the new directory and the file I'm testing with, move it to another computer, and test it there. And "stops responding" refers to the windows message "This application has stopped responding" when it crashes and tries to close it.

    And ah, I get the magic number thing now, ok. And thanks for the working directory. Does that need any particular header file? Oh, and is there a function to get the path from the file as well, or am I doing the best I can with that one?

    Edit: Wait, sizeof(uint32_t) == 4?

    Edit 2: _getcwd only gives me "C:\Documents and Settings\User", which is not what I wanted, lol. Is there a function that gives the full app path?
    Last edited by Molokai; 10-18-2009 at 02:04 PM.

  8. #8
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    sizeof(uint32_t) == 4, yes. What exactly did you expect? _getcwd requires <direct.h> I think. And the catch about the "full app path" is that it is irrelevant -- if you try to (say) open a file, that file will be opened in the current working directory regardless of where the app actually lives (unless you specify a full path).

  9. #9
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    Maybe this will make it more clear:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <dir.h> // for getcwd (Windows)
    
    /*
    	Test program:
    	- First invoke from command line. Note that the path is always correct.
    	- Now invoke by performing a drag/drop operation. Note that the path
    	is sometimes completely different, but the program name contains the 
    	correct path. 
    */
    	
    int main( int argc, char** argv )
    {
    	char path[ 1024 ];
    	printf( "Program Invocation: '%s'\n", argv[ 0 ] );
    	if( !getcwd( path, sizeof path ) )
    	{
    		perror( "'path' buffer too small" );
    		exit( 1 );
    	}
    	printf( "Current Directory: '%s'\n", path );
    	getchar( );
    	return 0;	
    }
    So the moral of the story is that in programming it helps to have the attitude "assume nothing, check everything" (and no, laziness is never a good excuse, either).

    >> Edit: Wait, sizeof(uint32_t) == 4?

    Not necessarily (but usually). Consider a system with 10-bit bytes, for example.

  10. #10
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by Sebastiani View Post
    Not necessarily (but usually). Consider a system with 10-bit bytes, for example.
    A system with 10-bit bytes would not be allowed (by the standard) to have a uint32_t datatype (assuming that the data types must be a multiple of the byte-size, which seems like a reasonable assumption even in this situation). (It would have a 40-bit uint_least32_t type, and it might have a uint40_t datatype.)

  11. #11
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    Quote Originally Posted by tabstop View Post
    A system with 10-bit bytes would not be allowed (by the standard) to have a uint32_t datatype (assuming that the data types must be a multiple of the byte-size, which seems like a reasonable assumption even in this situation). (It would have a 40-bit uint_least32_t type, and it might have a uint40_t datatype.)
    I see, so uint32_t is a standard typedef, then? Got it, and yes that makes sense. I was assuming it was his own typedef.

    @Molokai: If you want to get the path in all cases you could try something like this:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <dir.h> // for getcwd (Windows)
    
    /*
        Test program:
        - First invoke from command line. Note that the path is always correct.
        - Now invoke by performing a drag/drop operation. Note that the path
        is sometimes completely different, but the program name contains the 
        correct path. 
    */
    
    int get_program_path( char const* argv0, char* buf, size_t max )
    {
        char* end;
        int len;
        if( max == 0 )
            return 0;
        *buf = 0;
        if( ( end = strrchr( argv0, '\\' ) ) != 0 || ( end = strrchr( argv0, '/' ) ) != 0 )
        {
            len = end - argv0;
            if( len >= max )
                return 0;
            strncpy( buf, argv0, len );
            buf[ len ] = 0; // just in case strncpy neglects it
        }
        else if( !getcwd( buf, max ) )
            return 0;
        return 1;
    }
        
    int main( int argc, char** argv )
    {
        char path[ 1024 ];
        printf( "Program Invocation: '%s'\n", argv[ 0 ] );
        if( !get_program_path( argv[ 0 ], path, sizeof path ) )
        {
            perror( "could not obtain path" );
            exit( 1 );
        }
        printf( "Current Directory: '%s'\n", path );
        getchar( );
        return 0;    
    }

  12. #12
    Registered User
    Join Date
    May 2007
    Posts
    77
    Quote Originally Posted by tabstop View Post
    sizeof(uint32_t) == 4, yes. What exactly did you expect? _getcwd requires <direct.h> I think. And the catch about the "full app path" is that it is irrelevant -- if you try to (say) open a file, that file will be opened in the current working directory regardless of where the app actually lives (unless you specify a full path).
    Well, no, to me full app path is not irrelevant, because it's apparently not the directory it works in. This isn't an app I would have people install, it would just be downloaded and extracted, so I need the exact path the application resides in to get the right location to put the new converted files.

  13. #13
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by Molokai View Post
    Well, no, to me full app path is not irrelevant, because it's apparently not the directory it works in. This isn't an app I would have people install, it would just be downloaded and extracted, so I need the exact path the application resides in to get the right location to put the new converted files.
    Well, but that's what I meant: the "full app path" does not get you to the working directory. You can change the working directory with _setcwd, of course, once you've extracted it from your path.

  14. #14
    Registered User
    Join Date
    May 2007
    Posts
    77
    Well, that's what I'm saying, I don't want the current working directory, I mistyped that. I want the app's absolute path value.

  15. #15
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    So if this is always going to be run via drag-n-drop, then (1) document that fact and then (2) use that fact to say that the full path will be everything up to the last backslash in argv[0], which is what Sebastiani has been patiently typing and re-typing.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Numeric addresses for computers
    By great in forum C Programming
    Replies: 4
    Last Post: 08-23-2010, 11:53 AM
  2. Replies: 8
    Last Post: 05-07-2009, 11:31 AM
  3. Max/min function not working correctly
    By En-Motion in forum C++ Programming
    Replies: 6
    Last Post: 03-19-2009, 12:28 AM
  4. Matrix and vector operations on computers
    By DavidP in forum A Brief History of Cprogramming.com
    Replies: 11
    Last Post: 05-11-2004, 06:36 AM
  5. Stop errors on networked computers.
    By Bajanine in forum Networking/Device Communication
    Replies: 5
    Last Post: 11-02-2003, 01:00 PM