Re: Patch for pjsua2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Christian,

buy yourself a descent compiler!

Regards

-----Ursprüngliche Nachricht-----
Von: pjsip <pjsip-bounces@xxxxxxxxxxxxxxx> Im Auftrag von Christian Hoff
Gesendet: Montag, 24. September 2018 22:21
An: pjsip@xxxxxxxxxxxxxxx
Betreff: Re:  Patch for pjsua2

Hello Ming and all,

I have now done some research into SWIG and exceptions. After reading the SWIG documentation, I found out that SWIG indeed parses the C++ exception specification to figure out for what types of exceptions "catch(..)" should be generated (see http://www.swig.org/Doc3.0/SWIGDocumentation.html#SWIGPlus_exception_specifications
). Exception types that are specified in the exception specification are automatically propagated to the higher language (Java, C# etc.). Now that we need to remove the exception specification, we lose that capability. Thanks to Ming for the hint. However there is the possibility to adapt the SWIG interface file
("pjsip-apps/src/swig/pjsua2.i") and to add explicit "%catches" 
statements for each method that describe which types of exceptions the method could possibility throw. So I guess I need to rework my patch for this. I just hope that I will find some way to avoid having to annotate every method with "%catches".... I'll see. Or if someone knows how to do it, then I would be very grateful for a reply.


Best regards,

     Christian


On 24/09/18 12:43, Christian Hoff wrote:
> Hello Ming,
>
> thanks for your reply! I will have a look into the SWIG topic this 
> evening and will let you know my findings.
>
>
> Best regards,
>
>    Christian
>
>
> On 24/09/18 09:03, Ming wrote:
>> Hi Christian,
>>
>> We apologize for the delay in response. We're currently reviewing it 
>> and have also managed to do some testing. While for C/C++, this seems 
>> to be okay, we're still considering how to best handle this for our 
>> Android and Java (via SWIG binding), which may still need the "throw"
>> specifications in order for the developers to know that the functions 
>> may throw some exceptions.
>>
>> If you have any feedback regarding this, please let us know.
>>
>> Best regards,
>> Ming
>>
>> On Mon, Sep 24, 2018 at 2:32 PM Christian Hoff 
>> <christian_hoff@xxxxxxx> wrote:
>>> Hello all,
>>>
>>> some feedback for this patch is still appreciated. In general I am 
>>> getting somewhat frustrated about the collaboration with people who 
>>> want to contribute to the project. Already in the past other people 
>>> have posted patches here to this list, but did not get any response 
>>> - not even a response what needs to be changed for the patch to be 
>>> accepted. I think this is not a good mode of collaboration. Everyone 
>>> who posts a patch here to this mailing list has done this with the 
>>> intention to help the project PJSIP to move along (and has spent 
>>> much time developing the patch), so you should at least return the 
>>> favour and review the patch.
>>>
>>> This particular issue that I am trying to address with my patch 
>>> (that the PJSUA2 headers don't compile against C++17) is an issue 
>>> that you will eventually need to fix anyhow, so why not fix it right 
>>> now? As of now, modern versions of GCC use C++ 14 by default, it is 
>>> only a matter of time until C++ 17 will become the default and 
>>> people will start complaining that there are compile errors from the PJSUA2 headers.
>>>
>>> Besides this, my own app uses PJSUA2 with C++ 17 features, so I 
>>> would have to revert back to C++14 if this patch is not accepted upstream.
>>> This would be really unnecessary. So I would really appreciate a 
>>> patch review.
>>>
>>> Best regards,
>>>
>>>     Christian
>>>
>>>
>>> On 18/09/18 20:57, Christian Hoff wrote:
>>>> Hello all,
>>>> Hello Ming & Riza,
>>>>
>>>> do you have any feedback regarding this patch for C++ 17 support? I 
>>>> am also willing to rework it if required, but some feedback would 
>>>> be nice.
>>>>
>>>>
>>>> Thanks in advance and kind regards,
>>>>
>>>>      Christian
>>>>
>>>>
>>>> On 12/09/18 18:23, Christian Hoff wrote:
>>>>> Hello all,
>>>>>
>>>>> currently it is not possible to compile a pjsua2 application with
>>>>> C++17. The reason is that C++17 removes support for dynamic 
>>>>> C++exception
>>>>> specifications (that is "throws ..." declarations at the end of a 
>>>>> function). Unfortunately PJSUA2 makes heavy use of exception 
>>>>> specifications and most PJSUA2 functions are declared as "throw 
>>>>> (Error)" - meaning that they can throw an exception of type "Error"
>>>>> that the programmer is supposed to catch.
>>>>>
>>>>> The consequence is that a lot of compile errors can be observed 
>>>>> when compiling a pjsua2 application with C++17. The compile errors 
>>>>> look as
>>>>> follows:
>>>>>
>>>>> /usr/local/include/pjsua2/json.hpp:80:33: error: ISO C++17 does 
>>>>> not allow dynamic exception specifications
>>>>>       virtual string saveString() throw(Error);
>>>>>                                   ^~~~~
>>>>>
>>>>> The compile of the same application worked fine when using the 
>>>>> C++14 standard, but it got broken when I switched from C++14 to 
>>>>> the C++17 standard.
>>>>>
>>>>> See
>>>>> https://www.bfilipek.com/2017/05/cpp17-details-fixes-deprecation.h
>>>>> tml#removing-deprecated-exception-specifications-from-c17
>>>>>
>>>>> and
>>>>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0003r0.ht
>>>>> ml for more background about the removal of exception 
>>>>> specifications.
>>>>>
>>>>> To make the application compile again, I had to remove "throw(Error)"
>>>>> in the whole PJSUA2 source code and include files. This seems to 
>>>>> be the only viable and good fix for this issue. I wrote my own "sed"
>>>>> script to do that and made some cosmetic corrections afterwards. 
>>>>> Then I generated the attached patch file. Could you please apply 
>>>>> this patch to the code base or give me feedback what I still need 
>>>>> to improve? I verified that pjsip compiles fine with the patch applied.
>>>>>
>>>>> Thank you very much!
>>>>>
>>>>>
>>>>> Best regards,
>>>>>
>>>>>     Christian
>>> _______________________________________________
>>> Visit our blog: http://blog.pjsip.org
>>>
>>> pjsip mailing list
>>> pjsip@xxxxxxxxxxxxxxx
>>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>
>
> _______________________________________________
> Visit our blog: http://blog.pjsip.org
>
> pjsip mailing list
> pjsip@xxxxxxxxxxxxxxx
> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org


_______________________________________________
Visit our blog: http://blog.pjsip.org

pjsip mailing list
pjsip@xxxxxxxxxxxxxxx
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org


_______________________________________________
Visit our blog: http://blog.pjsip.org

pjsip mailing list
pjsip@xxxxxxxxxxxxxxx
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org




[Index of Archives]     [Asterisk Users]     [Asterisk App Development]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [Linux API]
  Powered by Linux