savestates_select_filename (savestates.c) is badly coded

[GoogleCodeExporter]

    static char fname[1024] = {0};

    [...]

    void savestates_select_filename(const char* fn)
    {
       if(strlen((char*)fn)>=1024)
           return;
       strcpy(fname, fn);
    }

- There's a useless cast (if there's some reason to have it, comment it).
- If strlen(fn) == 1024, strcpy will write the null terminator outside fname.
- The length of the filename is limited to 1024 characters (there's an issue 
already open for that).

Proposed solution to the first 2 problems:

    static char fname[1024] = {0};

    [...]

    void savestates_select_filename(const char* fn)
    {
       if(strlen(fn) < sizeof(fname))
           strcpy(fname, fn);
    }
Pruposed 

Original issue reported on code.google.com by GameZel...@gmail.com on 15 Feb 2011 at 10:08

[GoogleCodeExporter]

Original comment by richard...@gmail.com on 22 May 2011 at 4:34

• Added labels: Milestone-2.0

[GoogleCodeExporter]

> - There's a useless cast (if there's some reason to have it, comment it).

I would say that this is unnecessary. strlen is defined as "size_t strlen(const 
char *s);" --- Linux Programmer's Manual 1993-04-12

> - If strlen(fn) == 1024, strcpy will write the null terminator outside fname.

Why? 1024 is >= 1024 -> thus it will just return. Not that I like the function, 
but I don't see the problem right now.

Original comment by sven@narfation.org on 9 Jul 2011 at 6:08

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Patch is already done - 9b31e007e329. But feel free to clone the repos on 
bitbucket, fix those issues and make a pull request.

Original comment by sven@narfation.org on 9 Jul 2011 at 9:24

[GoogleCodeExporter]

Reposted this comment because I made it with the wrong main address. Sorry for 
the repost. This goes between the 2 messages of muellhierrein.

@muellhierrein: You're completely right. I don't know what I was thinking about 
when I found that "bug" and filled this bug report. Even my proposed solution 
is trivially equivalent to the original code. Probably I was looking at it too 
late and overlooked the = character... Even the title of the bug report isn't 
so smart. It's still funny that this was marked as Milestone-2.0.

Anyway, the changes required to fix the well-reported parts of the bug are 
trivial. Should make a patch for this and other small issues I found.

Original comment by GameZel...@gmail.com on 10 Jul 2011 at 10:18

[GoogleCodeExporter]

Original comment by s...@narfation.org on 29 Aug 2011 at 2:52

• Changed state: Fixed