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
