Re: [h323plus] Introducing PBoolean - API change alert!!!
Robert
Can you explain exactly in simple english 1. What you have changed. I assume it's changing every BOOL to PBOOLEAN in ptlib? 2. So everyone in every program derived from ptlib/pwlib has to change their program or when they compile against upcoming releases of ptlib their program will no longer function properly. Is that correct?
For what benefit? Compile time checks? moving from BOOL to ANSI bool? Though I have always wondered why BOOL was used in place of bool and globally replacing in h323plus is fairly easy however for the community as a whole who have developed complex applications from these libraries it's going to be extremely painful as backward compatibility is broken and worse with C++ virtual they will not even notice it has even occurred until they run the program. Not a very good situation at all.
For this reason I STRONGLY STRONGLY recommend you don't make this change. IMHO it is something inherent with pwlib/ptlib and far too painful this late in the game (after 10+ years?) for minimal benefit for everyone involved.
But of course, it already has been decided and the community have a week to agree and then it's done. This is what is truely unfortunate.
Simon
-----Original Message----- From: h323plus-bounces@lists.packetizer.com [mailto:h323plus-bounces@lists.packetizer.com]On Behalf Of Robert Jongbloed Sent: Monday, November 19, 2007 9:31 AM To: Opalvoip-devel@lists.sourceforge.net; h323plus@lists.packetizer.com Subject: [h323plus] Introducing PBoolean - API change alert!!!
If you have written any programs based on PTLib/OpenH323/H323plus/OPAL, please read this, IT'S IMPORTANT!!
Adam Butcher from Selex Communications submitted patches a while ago to change BOOL to PBoolean. The reason was so that PBoolean could be defined as the ANSI standard bool type and we get a whole pile of beneficial compile time checks. The process found a couple of places where a function had a BOOL return type but integer (tri-state) values were being returned!
Well, Craig and I had thought of this for a while as being a "good thing" to do, but lack of time has always been an issue. Now, as we have been presented with all the hack work, it seems that this is it's time.
Of course things are NEVER easy. The change has a nasty API issue that can silently stop user programs from working. It revolves around my #1 pet hate in C++, if you change the signature of a virtual function, overrides silently no longer override the virtual. For example:
//------ in the library
class OpalXXX
{
virtual void SomeFunction( PBoolean );
};
//------- in some unchanged dependent code in another application
class MyXXX : public OpalXXX
{
virtual void SomeFunction( BOOL );
};
The function MyXXX::SomeFunction() will not be called as it should be. And the stupid compiler (NONE of the compilers) give any warnings or indication there is an issue. Way back in the preliminary versions of C++ they did have a keyword "override" which would do exactly what we want, error if the base class does not have the function being overridden as virtual. The original designers (Stroustrup et al) took out, for presumably good reasons, though I cannot for the life of me think what! There is even a number proposals, e.g. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2108.html, to put a variants of it back and Microsoft have even done so, http://msdn2.microsoft.com/en-us/library/41w3sh1c.aspx, on their latest compiler.
Anyway we have to deal with existing compilers and this problem is really insidious, but we think the gains are worth the pain. And we have provided a work around for people.
So, what we need from you guys is please check out the PTLib and OPAL in the branches:
https://opalvoip.svn.sourceforge.net/svnroot/opalvoip/ptlib/branches/Bo oleanChange
https://opalvoip.svn.sourceforge.net/svnroot/opalvoip/opal/branches/Boo leanChange
and compile your programs against it. Now by default it will use the ANSI bool form of PBoolean, so any overrides of PTLib or OPAL functions must be changed to use PBoolean instead of BOOL. For those not using MFC, this is really easy, just do a complete search and replace everywhere of BOOL to PBoolean (or better yet, just bool) and that should do the job. Oh, you might like to search and replace TRUE to true and FALSE to false while you are at it, though you don't HAVE to do that.
Now, for those that simply do not wish to make these changes, we have a backward compatibility option: use the
--disable-ansi-bool
option with configure (both Unix and Win32) and PBoolean is defined as BOOL instead of bool. This should (in theory) be 100% compatible with the previous signatures. So no changes to applications based on PTLib need be made.
But we need you guys to CHECK THAT THIS IS OK!
We are currently proposing to merge it into the trunk on Monday 26th of November, we are happy to extend that if asked, but will not wait forever, the back merging is really painful!
P.S. A random note to developers, the PBoolean is only there for backward compatibility. It is recommended that any NEW functions all just use the bool keyword. Use lowercase true and false too.
P.P.S. Sorry about all the shouting ... J
Robert Jongbloed
OPAL/OpenH323 Architect and Co-founder.
Simon Horne wrote:
Robert
Can you explain exactly in simple english
- What you have changed. I assume it's changing every BOOL to PBOOLEAN in
ptlib?
Correct
- So everyone in every program derived from ptlib/pwlib has to change their
program or when they compile against upcoming releases of ptlib their program will no longer function properly. Is that correct?
No. BOOL still works
For what benefit? Compile time checks?
Yes
moving from BOOL to ANSI bool?
Yes
Though I have always wondered why BOOL was used in place of bool and globally replacing in h323plus is fairly easy however for the community as a whole who have developed complex applications from these libraries it's going to be extremely painful as backward compatibility is broken and worse with C++ virtual they will not even notice it has even occurred until they run the program. Not a very good situation at all.
Agreed. It is indeed unfortunate.
For this reason I STRONGLY STRONGLY recommend you don't make this change. IMHO it is something inherent with pwlib/ptlib and far too painful this late in the game (after 10+ years?) for minimal benefit for everyone involved.
That may be the case. But rather than just make the decision ourselves, we have decided to allow everyone to comment, as there are compelling arguments for both choices.
But of course, it already has been decided and the community have a week to agree and then it's done. This is what is truely unfortunate.
No, it has not been decided.
We agree with everything you have said, which is why the code is in a branch, and we are asking people to comment, rather than just dumping the code into head without notice.
One solution could be to make the default behavior be backwards compatible (through the correct default setting of the configure flag) rather than making the default to use the new bool. That way, all existing software need no changes, but people who want to get the benefits of the new bool can also do so.
What do you think of that proposal?
Craig
----------------------------------------------------------------------- Craig Southeren Post Increment – VoIP Consulting and Software craigs@postincrement.com.au www.postincrement.com.au
Phone: +61 243654666 ICQ: #86852844 Fax: +61 243656905 MSN: craig_southeren@hotmail.com Mobile: +61 417231046 Jabber: craigs@jabber.org
"Science is the poetry of reality." Richard Dawkins
participants (2)
-
Craig Southeren
-
Simon Horne