GNOME Bugzilla – Bug 421801
Yet another vsnprintf crash
Last modified: 2007-10-29 21:24:34 UTC
Just reported by Luc on an amd64 : <luc> #3 0x000000000047761f in gnomemeeting_warning_dialog_on_widget (parent=0x8cf350, <luc> key=0x489d30 "/apps/ekiga/devices/video/enable_preview", primary_text=<value optimized out>, <luc> format=0x486618 "%s") at gmdialog.c:495 The lines of code are : <luc> gnomemeeting_warning_dialog_on_widget (GTK_WINDOW (main_window), <luc> VIDEO_DEVICES_KEY "enable_preview", <luc> dialog_title, <luc> "%s", dialog_msg); Can't we just build the string to show, give it to gnomemeeting_warning_dialog_on_widget, then free it ? I'll try to have a look.
Looking back in the history
+ Trace 121391
Ok i've reproduce it: (gdb) thread apply all backtrace
+ Trace 121392
Thread 3 (Thread 1074272608 (LWP 11159))
Thread 1 (Thread 46912574814256 (LWP 11153))
One solution is to remove the use of vsprintf in the gnomemeeting_dialog_error_box. I've rewritten the code to be like this (patch follow) /* Translators: Do not translate MovingLogo and Picture */ static const char *error_code_strings[] = { NULL, N_("There was an error while opening the device. ...."), N_("Your video driver doesn't support the requested video format."), N_("Could not open the chosen channel."), N_("Your driver doesn't seem to support a...."), N_("Error while setting the frame rate."), N_("Error while setting the frame size."), }; /* If we want to open the fake device for a real error, and not because the user chose the Picture device */ gnomemeeting_threads_enter (); gm_history_window_insert (history_window, _("Couldn't open the video device")); dialog_title = g_strdup_printf (_("Error while opening video device %s"), (const char *) input_device); dialog_msg = g_strdup_printf(_("A moving logo w.... device.\n\n%s"), _(error_code_strings[error_code])); gnomemeeting_warning_dialog_on_widget (GTK_WINDOW (main_window), VIDEO_DEVICES_KEY "enable_preview", dialog_title, dialog_msg, NULL); g_free (dialog_msg); g_free (dialog_title);
Created attachment 85166 [details] [review] Fix segfault when displaying MovingLogo error dialog A better solution will be to replace all call to gnomemeeting_warning_dialog_on_widget with a safe string.
Hmmmm... two remarks about attachment #85166 [details] : - it removes a blank line after a "else {" ;-) - it doesn't give the same error message since it drops the first part of the error message. I'll try to work on the better solution : get rid of the va_list which isn't really used anyway!
Can you do that this week-end ? I'd like to release 2.0.8 ASAP. Why does the vsnprintf crash ? Isn't it the same bug than the one that was fixed Bug #418991 ?
Perhaps just adding a missing va_end will help. Also, that code is old. It never crashed before. Isn't that the same kind of bug with the PPLuginServiceDescriptor Bug #417801 ? Guys, we need to fix those two ASAP.
Created attachment 85202 [details] [review] Patch to fix the gnomemeeting_warning_dialog_on_widget function There was a va_end. I couldn't help but notice that there were other functions in there which use format strings : I didn't touch them since I was unsure the fix was as easy. Better do things stage by stage.
Now that I think about it, there was indeed a va_end, but not on all exit paths of the function... doesn't that lead to a corrupt stack ?
Created attachment 85214 [details] [review] Patch to fix the gnomemeeting_warning_dialog_on_widget function Ok, here is another simpler patch, which just makes sure we do the va_start after we decided for hid/don't hide : this makes the code do the va_start/va_end as a whole or not at all. Luc, since you can reproduce, could you please check if that is a correct fix ?
Comparing : http://svn.gnome.org/viewcvs/ekiga/tags/EKIGA_2_0_7/lib/gui/gmdialog.c?revision=4959&view=markup and http://svn.gnome.org/viewcvs/ekiga/trunk/lib/gui/gmdialog.c?view=markup Luc saw there were *two* vsnprintf calls! I still propose my patch, but indeed one of them has to go!
Created attachment 85215 [details] [review] Fix segfault when displaying MovingLogo error dialog (removing duplicate call) The bug is too stupid, two calls to vsnprintf() in the same function at least in the 2.0.7 release.
i think the two last patch need to be applied.
I committed the va_start push in gnome-2-14, trunk and H_Release branches. I didn't see the double vsnprintf in those branches.
*** Bug 417801 has been marked as a duplicate of this bug. ***
*** Bug 424312 has been marked as a duplicate of this bug. ***
*** Bug 424308 has been marked as a duplicate of this bug. ***
*** Bug 426603 has been marked as a duplicate of this bug. ***
*** Bug 419439 has been marked as a duplicate of this bug. ***
*** Bug 456405 has been marked as a duplicate of this bug. ***
*** Bug 456406 has been marked as a duplicate of this bug. ***
*** Bug 469194 has been marked as a duplicate of this bug. ***
*** Bug 469246 has been marked as a duplicate of this bug. ***
*** Bug 474495 has been marked as a duplicate of this bug. ***
*** Bug 476948 has been marked as a duplicate of this bug. ***
*** Bug 480584 has been marked as a duplicate of this bug. ***