Re: [Ekiga-devel-list] Crash in opal-gmconf-bridge.cpp
Matthias Schneider a écrit : > Quoting Damien Sandras <[EMAIL PROTECTED]>: > >> Le vendredi 11 avril 2008 à 11:26 +0200, Matthias Schneider a écrit : >>> Quoting Julien Puydt <[EMAIL PROTECTED]>: >>> Hi, again, there is a problem with the bridge : it gets something from gmconf, doesn't check before use, and triggers a floating point >> exception. I'm not sure whether the issue is in ekiga or in opal though : 0x08132c71 in GMManager::set_video_options (this=0x8303238, [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 877) { PWaitAndSignal m(_mutex); MakeUnique(); return m_info != NULL && m_info->SetOptionInteger(name, value); } (gdb) bt #0 0x08132c71 in GMManager::set_video_options (this=0x8303238, [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 #1 0x0814d3d8 in Opal::ConfBridge::on_property_changed (this=0x8319728, [EMAIL PROTECTED], entry=0x8290e60) at endpoints/opal-gmconf-bridge.cpp:127 The lines 126 and 127 of the bridge read like : options.maximum_frame_rate = gm_conf_entry_get_int (entry); manager.set_video_options (options); if for some reason zero is returned, then we have a crash. Snark >>> Sorry Snark, >>> this is my fault, I renamed that setting yesterday assuming its only used >> by the >>> vidinput-gmconf-bridge, where correct checks preventing your issue are >> being >>> done. I can offer to fix it tonight, or you can simply copy and paste the >>> section where its being read in the vidinput-gmconf-bridge to the opal >> bridge. >> I think the checks should be done in GMManager::set_video_options too. >> It would prevent crashes. (that is what I do). >> >> Btw, I cleaned a lot of things ;-) >> -- >> _ Damien Sandras > > Hm, I think I mentioned that already, but in my opinion the checks should be > done what is now the bridge. In my opinion there should not even be such a > thing like a bridge and its functionality be part of a service. In that way, > all methods that set stuff in the engine calle by what is now the bridge could > be private. However this is rather a post-3.00 idea. > > I have preferred to do all the checking in the bridge because this code > usually > get rather long and does not add anything helpful to understand the actual > code > in my cores. It will only make the core functions longer, something I would > like > to avoid. The checks have to be done where it fits. If an object has a method which gives a crash when receiving some arguments, then it's not the calls to the method which have to be protected, but the method itself which much be changed! You don't have to check what you get from gmconf if the method you call filters bad values correctly. Snark ___ Ekiga-devel-list mailing list Ekiga-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Re: [Ekiga-devel-list] Crash in opal-gmconf-bridge.cpp
Quoting Julien Puydt <[EMAIL PROTECTED]>: > Hi, > > again, there is a problem with the bridge : it gets something from > gmconf, doesn't check before use, and triggers a floating point exception. > > I'm not sure whether the issue is in ekiga or in opal though : > 0x08132c71 in GMManager::set_video_options (this=0x8303238, > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > 877 ) { PWaitAndSignal m(_mutex); MakeUnique(); return m_info != > NULL && m_info->SetOptionInteger(name, value); } > (gdb) bt > #0 0x08132c71 in GMManager::set_video_options (this=0x8303238, > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > #1 0x0814d3d8 in Opal::ConfBridge::on_property_changed (this=0x8319728, > [EMAIL PROTECTED], entry=0x8290e60) at > endpoints/opal-gmconf-bridge.cpp:127 > > The lines 126 and 127 of the bridge read like : > options.maximum_frame_rate = gm_conf_entry_get_int (entry); > manager.set_video_options (options); > if for some reason zero is returned, then we have a crash. > > Snark Sorry Snark, this is my fault, I renamed that setting yesterday assuming its only used by the vidinput-gmconf-bridge, where correct checks preventing your issue are being done. I can offer to fix it tonight, or you can simply copy and paste the section where its being read in the vidinput-gmconf-bridge to the opal bridge. Matthias This message was sent using IMP, the Internet Messaging Program. ___ Ekiga-devel-list mailing list Ekiga-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Re: [Ekiga-devel-list] Crash in opal-gmconf-bridge.cpp
Quoting Damien Sandras <[EMAIL PROTECTED]>: > Le vendredi 11 avril 2008 à 11:26 +0200, Matthias Schneider a écrit : > > Quoting Julien Puydt <[EMAIL PROTECTED]>: > > > > > Hi, > > > > > > again, there is a problem with the bridge : it gets something from > > > gmconf, doesn't check before use, and triggers a floating point > exception. > > > > > > I'm not sure whether the issue is in ekiga or in opal though : > > > 0x08132c71 in GMManager::set_video_options (this=0x8303238, > > > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > > > 877 ) { PWaitAndSignal m(_mutex); MakeUnique(); return m_info != > > > NULL && m_info->SetOptionInteger(name, value); } > > > (gdb) bt > > > #0 0x08132c71 in GMManager::set_video_options (this=0x8303238, > > > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > > > #1 0x0814d3d8 in Opal::ConfBridge::on_property_changed (this=0x8319728, > > > [EMAIL PROTECTED], entry=0x8290e60) at > > > endpoints/opal-gmconf-bridge.cpp:127 > > > > > > The lines 126 and 127 of the bridge read like : > > > options.maximum_frame_rate = gm_conf_entry_get_int (entry); > > > manager.set_video_options (options); > > > if for some reason zero is returned, then we have a crash. > > > > > > Snark > > > > Sorry Snark, > > this is my fault, I renamed that setting yesterday assuming its only used > by the > > vidinput-gmconf-bridge, where correct checks preventing your issue are > being > > done. I can offer to fix it tonight, or you can simply copy and paste the > > section where its being read in the vidinput-gmconf-bridge to the opal > bridge. > > > > I think the checks should be done in GMManager::set_video_options too. > It would prevent crashes. (that is what I do). > > Btw, I cleaned a lot of things ;-) > -- > _ Damien Sandras Hm, I think I mentioned that already, but in my opinion the checks should be done what is now the bridge. In my opinion there should not even be such a thing like a bridge and its functionality be part of a service. In that way, all methods that set stuff in the engine calle by what is now the bridge could be private. However this is rather a post-3.00 idea. I have preferred to do all the checking in the bridge because this code usually get rather long and does not add anything helpful to understand the actual code in my cores. It will only make the core functions longer, something I would like to avoid. Matthias This message was sent using IMP, the Internet Messaging Program. ___ Ekiga-devel-list mailing list Ekiga-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Re: [Ekiga-devel-list] Crash in opal-gmconf-bridge.cpp
Le vendredi 11 avril 2008 à 11:26 +0200, Matthias Schneider a écrit : > Quoting Julien Puydt <[EMAIL PROTECTED]>: > > > Hi, > > > > again, there is a problem with the bridge : it gets something from > > gmconf, doesn't check before use, and triggers a floating point exception. > > > > I'm not sure whether the issue is in ekiga or in opal though : > > 0x08132c71 in GMManager::set_video_options (this=0x8303238, > > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > > 877 ) { PWaitAndSignal m(_mutex); MakeUnique(); return m_info != > > NULL && m_info->SetOptionInteger(name, value); } > > (gdb) bt > > #0 0x08132c71 in GMManager::set_video_options (this=0x8303238, > > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > > #1 0x0814d3d8 in Opal::ConfBridge::on_property_changed (this=0x8319728, > > [EMAIL PROTECTED], entry=0x8290e60) at > > endpoints/opal-gmconf-bridge.cpp:127 > > > > The lines 126 and 127 of the bridge read like : > > options.maximum_frame_rate = gm_conf_entry_get_int (entry); > > manager.set_video_options (options); > > if for some reason zero is returned, then we have a crash. > > > > Snark > > Sorry Snark, > this is my fault, I renamed that setting yesterday assuming its only used by > the > vidinput-gmconf-bridge, where correct checks preventing your issue are being > done. I can offer to fix it tonight, or you can simply copy and paste the > section where its being read in the vidinput-gmconf-bridge to the opal bridge. > I think the checks should be done in GMManager::set_video_options too. It would prevent crashes. (that is what I do). Btw, I cleaned a lot of things ;-) -- _ Damien Sandras (o- //\Ekiga Softphone : http://www.ekiga.org/ v_/_ NOVACOM : http://www.novacom.be/ FOSDEM : http://www.fosdem.org/ SIP Phone : sip:[EMAIL PROTECTED] ___ Ekiga-devel-list mailing list Ekiga-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Re: [Ekiga-devel-list] Crash in opal-gmconf-bridge.cpp
Quoting Julien Puydt <[EMAIL PROTECTED]>: > Hi, > > again, there is a problem with the bridge : it gets something from > gmconf, doesn't check before use, and triggers a floating point exception. > > I'm not sure whether the issue is in ekiga or in opal though : > 0x08132c71 in GMManager::set_video_options (this=0x8303238, > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > 877 ) { PWaitAndSignal m(_mutex); MakeUnique(); return m_info != > NULL && m_info->SetOptionInteger(name, value); } > (gdb) bt > #0 0x08132c71 in GMManager::set_video_options (this=0x8303238, > [EMAIL PROTECTED]) at /usr/include/opal/opal/mediafmt.h:877 > #1 0x0814d3d8 in Opal::ConfBridge::on_property_changed (this=0x8319728, > [EMAIL PROTECTED], entry=0x8290e60) at > endpoints/opal-gmconf-bridge.cpp:127 > > The lines 126 and 127 of the bridge read like : > options.maximum_frame_rate = gm_conf_entry_get_int (entry); > manager.set_video_options (options); > if for some reason zero is returned, then we have a crash. > > Snark Its fixed now. Sorry for the inconvenience. Matthias This message was sent using IMP, the Internet Messaging Program. ___ Ekiga-devel-list mailing list Ekiga-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Re: [Ekiga-devel-list] Crash in opal-gmconf-bridge.cpp
Le vendredi 11 avril 2008 à 11:56 +0200, Matthias Schneider a écrit : > Quoting Damien Sandras <[EMAIL PROTECTED]>: [...] > > > > I think the checks should be done in GMManager::set_video_options too. > > It would prevent crashes. (that is what I do). [...] > I have preferred to do all the checking in the bridge because this code > usually > get rather long and does not add anything helpful to understand the actual > code > in my cores. It will only make the core functions longer, something I would > like > to avoid. I'm using code like this to ensure sane values: SetAudioJitterDelay (PMAX (min_val, 20), PMIN (max_val, 1000)); -- _ Damien Sandras (o- //\Ekiga Softphone : http://www.ekiga.org/ v_/_ NOVACOM : http://www.novacom.be/ FOSDEM : http://www.fosdem.org/ SIP Phone : sip:[EMAIL PROTECTED] ___ Ekiga-devel-list mailing list Ekiga-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Re: [Ekiga-devel-list] Crash in opal-gmconf-bridge.cpp
Damien Sandras a écrit : > I'm using code like this to ensure sane values: > SetAudioJitterDelay (PMAX (min_val, 20), PMIN (max_val, 1000)); That is wrong : you're protecting the call to SetAudioJitterDelay, not the method itself! If I have a method which receives an integer which shouldn't be zero, then that method should check for it itself, not each and every use of it! Snark ___ Ekiga-devel-list mailing list Ekiga-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/ekiga-devel-list