From: Andrej N. G. <an...@re...> - 2012-05-21 21:12:15
|
Hello! I need help with examining of bug # 3462452. Problem happens in libfm. Exactly what happens: 1) libfm detects that bookmark isn't directory but file; 2) it says: ** (pcmanfm:21245): DEBUG: FmJob error: The specified directory is not valid 3) it does not change directory and tries to free invalid object it got: #12 0x00637d78 in fm_dir_list_job_finalize (object=0x9e826c0) at job/fm-dir-list-job.c:91 91 fm_file_info_unref(self->dir_fi); #10 0x00630f8f in fm_file_info_unref (fi=0xb6b18f88) at base/fm-file-info.c:297 297 fm_file_info_clear( fi ); #9 0x00630f2c in fm_file_info_clear (fi=0xb6b18f88) at base/fm-file-info.c:261 261 g_free(fi->disp_name); where: (gdb) p *fi $2 = {path = 0x0, mode = 0, {fs_id = 0x807 <Address 0x807 out of bounds>, dev = 2055}, uid = 500, gid = 70, size = 0, mtime = 1336601702, atime = 1337631971, blksize = 0, blocks = 0, disp_name = 0xad303e11 "a.txt", collate_key = 0x0, disp_size = 0x0, disp_mtime = 0x0, type = 0x0, icon = 0x0, target = 0x0, n_ref = 0} but fi->disp_name seems to be out of scope so program aborted (SIGABRT): *** glibc detected *** pcmanfm: free(): invalid pointer: 0xad303e11 *** I think the problem is in two questionable places: src/base/fm-file-info.c:91: if( strcmp(tmp, fi->path->name) == 0 ) fi->disp_name = fi->path->name; else fi->disp_name = g_strdup(tmp); src/base/fm-file-info.c:330: if(src->disp_name == src->path->name) fi->disp_name = src->disp_name; else fi->disp_name = g_strdup(src->disp_name); What I cannot find out and want help - why it is just set? I think it should be g_strdup() unconditionally because it will be freed later. Or have I missed the point? Anyway one of those places makes that crash so should be fixed. With best wishes. Andriy. |
From: Andrej N. G. <an...@re...> - 2012-05-21 21:24:52
|
Hello! >I think the problem is in two questionable places: >src/base/fm-file-info.c:91: > if( strcmp(tmp, fi->path->name) == 0 ) > fi->disp_name = fi->path->name; > else > fi->disp_name = g_strdup(tmp); >src/base/fm-file-info.c:330: > if(src->disp_name == src->path->name) > fi->disp_name = src->disp_name; > else > fi->disp_name = g_strdup(src->disp_name); After some thinking I've found that first place is handled in appropriate places but second cannot be. I'll send patch to tracker then. Cheers! Andriy. |
From: PCMan <pcm...@gm...> - 2012-05-21 23:59:31
|
No, it's not a bug. It's a dirty trick to reduce memory usage. For filenames in English, their real names and their UTF-8 display names are completely the same. So there is no need to store two copies of the same string. That's the purpose of this hack, to share real base names and display names whenever possible. This is not a bug, but I did not write good comments for this previously. :-( On Tue, May 22, 2012 at 5:24 AM, Andrej N. Gritsenko <an...@re...>wrote: > Hello! > > >I think the problem is in two questionable places: > > >src/base/fm-file-info.c:91: > > > if( strcmp(tmp, fi->path->name) == 0 ) > > fi->disp_name = fi->path->name; > > else > > fi->disp_name = g_strdup(tmp); > > >src/base/fm-file-info.c:330: > > > if(src->disp_name == src->path->name) > > fi->disp_name = src->disp_name; > > else > > fi->disp_name = g_strdup(src->disp_name); > > After some thinking I've found that first place is handled in appropriate > places but second cannot be. I'll send patch to tracker then. > > Cheers! > Andriy. > > > ------------------------------------------------------------------------------ > Live Security Virtual Conference > Exclusive live event will cover all the ways today's security and > threat landscape has changed and how IT managers can respond. Discussions > will include endpoint security, mobile security and the latest in malware > threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ > _______________________________________________ > Pcmanfm-develop mailing list > Pcm...@li... > https://lists.sourceforge.net/lists/listinfo/pcmanfm-develop > |
From: Andrej N. G. <an...@re...> - 2012-05-22 00:15:17
|
Hello! PCMan has written on Tuesday, 22 May, at 7:59: >No, it's not a bug. It's a dirty trick to reduce memory usage. >For filenames in English, their real names and their UTF-8 display names >are completely the same. So there is no need to store two copies of the >same string. >That's the purpose of this hack, to share real base names and display names >whenever possible. This is not a bug, but I did not write good comments for >this previously. :-( That's OK, I've found that trick and left it alone. Bug was caused by another few issues, I've fixed them all, patch is in Patches tracker, see #3528712. Cheers! Andriy. |
From: PCMan <pcm...@gm...> - 2012-05-22 01:03:19
|
Thanks for the fixes. So, you're the user "lstranger"? Thanks a lot for helping the project. When the whole filename contains English strings without digits or special chars, the collect key is totally the same as the file basename. So, I did the same trick for collate_key. That's why it's not copied unconditionally. I need to recheck this part, though. On Tue, May 22, 2012 at 8:15 AM, Andrej N. Gritsenko <an...@re...>wrote: > Hello! > > PCMan has written on Tuesday, 22 May, at 7:59: > >No, it's not a bug. It's a dirty trick to reduce memory usage. > >For filenames in English, their real names and their UTF-8 display names > >are completely the same. So there is no need to store two copies of the > >same string. > >That's the purpose of this hack, to share real base names and display > names > >whenever possible. This is not a bug, but I did not write good comments > for > >this previously. :-( > > That's OK, I've found that trick and left it alone. Bug was caused by > another few issues, I've fixed them all, patch is in Patches tracker, see > #3528712. > > Cheers! > Andriy. > > > ------------------------------------------------------------------------------ > Live Security Virtual Conference > Exclusive live event will cover all the ways today's security and > threat landscape has changed and how IT managers can respond. Discussions > will include endpoint security, mobile security and the latest in malware > threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ > _______________________________________________ > Pcmanfm-develop mailing list > Pcm...@li... > https://lists.sourceforge.net/lists/listinfo/pcmanfm-develop > |
From: Andrej N. G. <an...@re...> - 2012-05-22 01:55:36
|
Hello! PCMan has written on Tuesday, 22 May, at 9:03: >Thanks for the fixes. So, you're the user "lstranger"? >Thanks a lot for helping the project. >When the whole filename contains English strings without digits or special >chars, the collect key is totally the same as the file basename. So, I did >the same trick for collate_key. That's why it's not copied unconditionally. >I need to recheck this part, though. It cannot be bound to name which is bound to another name or else you should realloc it each time the second name is changed and that check will consume more resources than that once-done allocation does. Not counting the fact that disp_name can also be changed elsewhere so it's always potential bug (remember it already was in mentioned bug report) and also it's very unlikely the same string as people rarely have files named with English-lower-case letters, especially not Englishmen. So at least leave that collate_key be always allocated, it'll save you a lot of troubles in the future. BTW, that memory consuming is very low compared to all other stuff. Cheers! Andriy. |