From: Hartwig D. <har...@gm...> - 2006-04-25 20:35:48
|
Dear Developers, I have just uploaded two patches to the patches section, which remove the file ithreads.h and change all users to directly include <pthreads.h> and its functions instead. ithreads.h did not provide any functionality but jus= t re-defined the "pthread_" prefix to "ithread_". Hence I think it does not provide any benefits and only hides that libupnp uses posix threads.If you consider this change usefull, please apply, Cheers, Hartwig |
From: <qx...@gm...> - 2006-04-26 07:11:06
|
Hi, there is one big problem with this change: the current variant offers an easy way to port the library to other operation systems that do NOT use the pthreads-thingy (like Windows). Currently the adaption has to be done in ithreads.h only, using this change developers have to modify the code everywhere. Michael > --- Ursprüngliche Nachricht --- > Von: "Hartwig Deneke" <har...@gm...> > An: upn...@li... > Betreff: [upnp-sdk-dev] Patches: Remove ithread.h header and change users > over to pthreads > Datum: Tue, 25 Apr 2006 22:35:41 +0200 > > Dear Developers, > > I have just uploaded two patches to the patches section, which remove the > file ithreads.h and change all users to directly include <pthreads.h> and > its functions instead. ithreads.h did not provide any functionality but > just > re-defined the "pthread_" prefix to "ithread_". Hence I think it does not > provide any benefits and only hides that libupnp uses posix threads.If you > consider this change usefull, please apply, > Cheers, > Hartwig > -- "Feel free" - 10 GB Mailbox, 100 FreeSMS/Monat ... Jetzt GMX TopMail testen: http://www.gmx.net/de/go/topmail |
From: Hartwig D. <har...@gm...> - 2006-04-26 14:52:28
|
2006/4/26, qxc@ <qx...@gm...>... > > there is one big problem with this change: the current variant offers an > easy way to port the library to other operation systems that do NOT use > the > pthreads-thingy (like Windows). > > Currently the adaption has to be done in ithreads.h only, using this > change > developers have to modify the code everywhere. > Hi, I do not agree with this argument. libupnp currently depends on posix threads functionality, renaming functions from pthread_xxx to ithread_xxx does nothing to change this. No additional functionality is provided by the ithread header!! Hence, the fact that libupnp requires posix threads (and uses its data-structures) is hidden, which is IMHO a bad thing. If libupnp was using actually using a portable threads library, it would be a completely different story. Once somebody decides to port libupnp to a platform without posix threads, the functionality of the required subset of pthreads has to be provided for that platform. Why not implement it as a separate library, preferrably called libpthread and providing a header pthread.h (it could still be distributed with the libupnp source code, but at least for me, this seems like an independent task)? BTW, there is a pthread library for win32, see http://sourceware.org/pthreads-win32/, so no need to worry about windows. BTW, there is something else to consider about these patches: projects usin= g libupnp might have to be changed, if they use ithread.h, thus this is a user-visible API change (checked with gmediaserver, which does use this header, if only in a few places). Hartwig |
From: <qx...@gm...> - 2006-04-26 15:05:11
|
The point is, that there must be a reson for the Intel developers to do it in that way. What else than the preparation for porting it to other systems should it be? On the other hand: what do we win with that change? I think nothing, the code will not become smaller because these defines are replaced by the preprocessor completely. What do we lose? In my opinion a (probably small) piece of compatibility. Beside of that you pointed to an other big problem with that patch: it is a user-visible API change that is not good for compatibility. Mike On Wed, 26 Apr 2006 16:45:05 +0200, Hartwig Deneke <har...@gm...> wrote: > 2006/4/26, qxc@ <qx...@gm...>... >> >> there is one big problem with this change: the current variant offers an >> easy way to port the library to other operation systems that do NOT use >> the >> pthreads-thingy (like Windows). >> >> Currently the adaption has to be done in ithreads.h only, using this >> change >> developers have to modify the code everywhere. >> > Hi, > > I do not agree with this argument. libupnp currently depends on posix > threads functionality, renaming functions from pthread_xxx to ithread_xxx > does nothing to change this. No additional functionality is provided by > the > ithread header!! Hence, the fact that libupnp requires posix threads (and > uses its data-structures) is hidden, which is IMHO a bad thing. If > libupnp > was using actually using a portable threads library, it would be a > completely different story. > > Once somebody decides to port libupnp to a platform without posix > threads, > the functionality of the required subset of pthreads has to be provided > for > that platform. Why not implement it as a separate library, preferrably > called libpthread and providing a header pthread.h (it could still be > distributed with the libupnp source code, but at least for me, this seems > like an independent task)? BTW, there is a pthread library for win32, > see > http://sourceware.org/pthreads-win32/, so no need to worry about windows. > > BTW, there is something else to consider about these patches: projects > using > libupnp might have to be changed, if they use ithread.h, thus this is a > user-visible API change (checked with gmediaserver, which does use this > header, if only in a few places). > > Hartwig |
From: Hartwig D. <har...@gm...> - 2006-04-27 16:19:58
|
Hi, as the patches somehow disappeared, probably because anonymous upload of patches is not allowed, I have re-uploaded the changes required for getting rid of ithreads. This time, I have split this change up into 3 patches this time. The first = 2 patches change users of the ithread header from the threadutils and the upn= p subdirectory to directly use pthreads, and the last one actually removes this header. Thus, the maintainers have the choice to actually keep around this header file, so external applications using it can continue so for a while, in order not to suddenly break the API (no idea whether the API of libupnp is considered stable already). Any opinions apart from Mike on this subject? Also, a few replies to Mike's statements 2006/4/26, qx...@gm... <qx...@gm...>: > The point is, that there must be a reson for the Intel developers to do i= t > > in that way. What else than the preparation for porting it to other > systems should it be? This is wild guessing, but maybe Intel did have pthreads implementations fo= r other platforms such as windows or vxworks, but decided to not release them= ? Still, I do think they should have made it a separate libpthread then. On the other hand: what do we win with that change? I think nothing, the > code will not become smaller because these defines are replaced by the > preprocessor completely. Well, I believe getting rid of code obfuscation is always worth it. Also, code does bit-rot if it is unmaintained. Examples of this are already present, as ithread.h currently contains broke= n defines (ithread_mutexattr_setkind_np vs. pthread_mutexattr_settype available on my SUSE Linux 9.3), and does miss functionality available in m= y current pthreads package (function pthread_testcancel, macro PTHREAD_MUTEX_INITIALIZER). Beside of that you pointed to an other big problem with that patch: it is > a user-visible API change that is not good for compatibility. The problem of a user-visible API change can be aleviatedby carrying around the ithread.h file for a while, possibly adding a big warning that it is going to be removed in the future. Hartwig |
From: <tur...@ea...> - 2006-04-27 20:09:04
|
On Thursday 27 April 2006 18:19, Hartwig Deneke wrote: > > Well, I believe getting rid of code obfuscation is always worth it. Also, > code does bit-rot if it is unmaintained. > Examples of this are already present, as ithread.h currently contains > broken defines (ithread_mutexattr_setkind_np vs. pthread_mutexattr_settype > available on my SUSE Linux 9.3), and does miss functionality available in > my current pthreads package (function pthread_testcancel, macro > PTHREAD_MUTEX_INITIALIZER). I must admit that the ithread is a bit broken. It is incomplete (missing above initializers) which cause using programmes to be more complex than necessary. Also, it exports some non-portable definitions (*_np) which are not available everywhere. So I had trouble when porting the library to FreeBSD : some of the _np definitions are not available exactly in the same way, but are used in the "sample" programs. So should I make a compatible implementation of these definitions on FreeBSD ? It would have been better not to export them in the first place ... > > > a user-visible API change that is not good for compatibility. > > The problem of a user-visible API change can be aleviatedby carrying around > the ithread.h file for a while, possibly adding a big warning that it is > going to be removed in the future. > I don't know. What do other people think ? |
From: <qx...@gm...> - 2006-04-30 13:33:14
|
>> > a user-visible API change that is not good for compatibility. >> >> The problem of a user-visible API change can be aleviatedby carrying >> around >> the ithread.h file for a while, possibly adding a big warning that it is >> going to be removed in the future. > > I don't know. > What do other people think ? > I think we should have a decision from the maintainer. |
From: <tur...@ea...> - 2006-04-30 16:26:57
|
On Thursday 27 April 2006 18:19, Hartwig Deneke wrote: > I have re-uploaded the changes required for getting > rid of ithreads. > This time, I have split this change up into 3 patches this time. btw, you don't need to create a new tracker item each time : one is=20 sufficient, you can attach multiple files if you wish anyway. R=E9mi |