Page 1 of 1

[Fixed] Fix assertion fault on VS2008 debug build

Posted: Tue Jan 04, 2011 2:32 pm
by gocha
Prevented refering erased iterator.

Code: Select all

diff --git a/conffile.cpp b/conffile.cpp
index 94983e7..5f437ab 100644
--- a/conffile.cpp
+++ b/conffile.cpp
@@ -459,19 +459,21 @@ char *ConfigFile::GetStringDup(const char *key, const char *def){
 bool ConfigFile::SetString(const char *key, string val, const char *comment){
     set<ConfigEntry, ConfigEntry::key_less>::iterator i;
     bool ret=false;
+    bool found;
 
     ConfigEntry e(key, val);
 	if(comment && *comment) e.comment = comment;
 	e.used=true;
 
     i=data.find(e);
-    if(i!=data.end()){
+    found=(i==data.end());
+    if(!found){
         e.line=i->line;
         data.erase(e);
 		sectionSizes.DecreaseSectionSize(e.section);
         ret=true;
     }
-	if((i==data.end() && (!alphaSort || timeSort)) || (!alphaSort && timeSort))
+	if((found && (!alphaSort || timeSort)) || (!alphaSort && timeSort))
 		e.line = linectr++;
 
     data.insert(e);

Posted: Tue Jan 04, 2011 2:35 pm
by gocha
Also, current snes9xgit master might cause another assertion error, when opening a movie from movie open dialog, if it's not my fault. (I guess it's because both fd and stream points the same file description. I don't know how it should be fixed.)

Posted: Fri Jan 07, 2011 1:12 pm
by BearOso
Ok, committed.
gocha wrote:Also, current snes9xgit master might cause another assertion error, when opening a movie from movie open dialog, if it's not my fault. (I guess it's because both fd and stream points the same file description. I don't know how it should be fixed.)
This is visual studio being too pedantic. It's perfectly OK to have two different references to the same file descriptor.

Posted: Fri Jan 07, 2011 3:34 pm
by OV2
It's not only an assertion, it's a crash on every movie playback. I think the problem was caused during the dup and access call removal. Removing the fclose call after CLOSE_STREAM in S9xMovieOpen stops the crash. IIRC closing a stream also closes the associated file descriptor - that might be the problem here.

Posted: Fri Jan 07, 2011 5:27 pm
by BearOso
Doh. In spec, double-closing a file descriptor definitely shouldn't crash. We also can't leave the FILE type stream there because it's a leak.

Ok, so the problem here is that we need an integer file number to pass to REOPEN_STREAM. We can't just OPEN_STREAM because, when using zlib, seeking is proportional to the compressed size, and the header is uncompressed. We can't use "dup" because it's not standard C, and we can't use "open" because it's also not standard C. And ideas?

Posted: Sat Jan 08, 2011 12:29 pm
by BearOso
Ok, a little research shows that the FILE objects might be managed by the C library, so fclose won't leave it intact. I've checked it in valgrind, any no leak comes from there, so it should be fine.

Posted: Sat Jan 08, 2011 3:03 pm
by gocha
BearOso wrote:Ok, a little research shows that the FILE objects might be managed by the C library, so fclose won't leave it intact. I've checked it in valgrind, any no leak comes from there, so it should be fine.
I checked your update and it worked fine. Thanks.

Posted: Sat Jan 08, 2011 7:34 pm
by Camo_Yoshi
I look at this topic in a tab and see...

Image

And go: O_o'

Posted: Sat Jan 08, 2011 8:45 pm
by the_randomizer
Camo_Yoshi wrote:I look at this topic in a tab and see...

Image

And go: O_o'
Wow. Simply amazing. :shock: Just like the poorly abbreviated words for Assistant Manager.