Possible error in ini_search_by_crc (main/romcache.c)

[GoogleCodeExporter]

Describe your system:
 - Linux distribution: FC9
 - Mupen64Plus version: trunk

I had some problems with the calculation of MD5 (Ref my comment to 
issue#89) so I added a line to search after CRC1 and CRC2 if MD5 failed:
Index: main/romcache.c
===================================================================
--- main/romcache.c     (revisjon 1096)
+++ main/romcache.c     (arbeidskopi)
@@ -326,6 +326,9 @@
     entry->crc1 = sl(*(unsigned int*)(localrom+0x10));
     entry->crc2 = sl(*(unsigned int*)(localrom+0x14));

+    if (entry->inientry->goodname == NULL)
+        entry->inientry = ini_search_by_crc(entry->crc1, entry->crc2);
+
     /* Internal name is encoded in SHIFT-JIS. Attempt to convert to UTF-8 
so that
     GUIs and (ideally) Rice can use this for Japanese titles in a moderm 
environment. */
     iconv_t conversion = iconv_open ("UTF-8", "SHIFT-JIS");

In function ini_search_by_crc I had to do the following modifications to 
make it work.

Index: main/romcache.c
===================================================================
--- main/romcache.c     (revisjon 1096)
+++ main/romcache.c     (arbeidskopi)
@@ -942,7 +945,7 @@
     if(g_romdatabase.comment==NULL)
         return &empty_entry;
     romdatabase_search* search;
-    search = g_romdatabase.crc_lists[(unsigned short)crc1];
+    search = g_romdatabase.crc_lists[crc1>>24];

     while (search != NULL && search->entry.crc1 != crc1 && search-
>entry.crc2 != crc2)
         search = search->next_crc;

As far as I can see the crc_lists is indexed on the first byte (MSB) of 
CRC1. As far as I know the '(unsigned short)crc1' statement will give the 2 
last bytes (LSB) of CRC1.

The solution would be to use 'crc1>>24' instead. This solved it for me and 
I was able to find my entry in the database.

I can see that 'ini_search_by_crc' is used in open_rom (main/rom.c). If it 
is working here I have not tested/investegated, but if it is I rest my 
case...

I have not thought about any possible side affect to this. Maybe it will 
brak anything else? Big/Little endian issues?

Original issue reported on code.google.com by olejl77@gmail.com on 10 Oct 2008 at 9:35

[GoogleCodeExporter]

ini_search_by_crc is depreciated and only used as a fallback in rom.c and by 
the 
cheat subsystem which is in the process of being rewritten. 

I fixed the core issue in issue 89. I'm fairly sure the code does work, but if 
not, 
>> has endian issues. The real solution is either to remove the function 
altogether 
or change the API to passing an unsigned char as that's the size of the CRCs.

Original comment by sknau...@wesleyan.edu on 11 Oct 2008 at 3:12

[GoogleCodeExporter]

I agree it is only used as fallback in rom.c, but my opinion is still that it is
failing. I have now tested with a faulty .zip archive. The crc1 and crc2 are
successfully extracted from the rom header, but still the game is not found in 
the
ini file.

I see this line printed on the terminal (rom.c:581):
            printf("%s\n", ROM_SETTINGS.goodname);

Not sure what you mean with "I fixed the core issue in issue 89".

Tried to make a little more robust patch:

Index: main/romcache.c
===================================================================
--- main/romcache.c (revision 1102)
+++ main/romcache.c (working copy)
@@ -939,11 +939,20 @@

 romdatabase_entry* ini_search_by_crc(unsigned int crc1, unsigned int crc2)
 {
+    unsigned char buffer[10];
+    char temp[3];
+    temp[2] = '\0';
+
     if(g_romdatabase.comment==NULL) 
         return &empty_entry;
     romdatabase_search* search;
-    search = g_romdatabase.crc_lists[(unsigned short)crc1];

+    sprintf(buffer, "%x", crc1);
+    temp[0] = buffer[0];
+    temp[1] = buffer[1];
+
+    search = g_romdatabase.crc_lists[hexconvert(temp)];
+
     while (search != NULL && search->entry.crc1 != crc1 && search->entry.crc2 != crc2)
         search = search->next_crc;

Original comment by olejl77@gmail.com on 11 Oct 2008 at 7:37

[GoogleCodeExporter]

I meant I fixed the .zip problem in issue 89.

Anyway... I did some more testing and yes, there is a bug. I applied a similar 
patch in r1108

Original comment by sknau...@wesleyan.edu on 11 Oct 2008 at 10:50

• Changed state: Fixed

[GoogleCodeExporter]

Fix verified  OK.

Original comment by olejl77@gmail.com on 12 Oct 2008 at 5:49