From: John D. <jd...@re...> - 2006-03-15 19:55:14
|
On Thu, 2006-03-09 at 23:02 +0100, R=C3=A9mi wrote: > John, >=20 > > First of all Remi, thank you for your work and releasing version 1.3.= 1. > > I've downloaded it and did some testing. I also reviewed the work you > > did to convert building with automake, libtool, pkgconfig etc. It loo= ks > > excellent and is a huge improvement! > > =20 > thanks ! >=20 > > On the issue of installation, I see that the libraries for threadutil > > and ixml are installed in the system in parallel with upnp. I wonder = if > > in the long run that's a good idea. (...) > > At some point it might be > > worthwhile to link these libraries into libupnp and hide their symbol= s. > > =20 > except that ixml has its own documented interface (see the libupnp-doc=20 > package), and normally is used directly by programs interfacing with=20 > libupnp.I think it would not be practical to hide it. May be merge it,=20 > but that would break programs linking with the 3 libraries. Good points, sounds fine to me. >=20 > > > > In the UPnP SDK ixmlPrintDocument had been #define'd to be ixmlPrintN= ode > > which prints XML elements. The fix I believe is to have to independen= t > > functions, ixmlPrintDocument and ixmlPrintNode (same for the toString > > versions) where the Document functions include the prolog and the Nod= e > > functions omit it. The attached patch (which I'll also post on the SF > > site) creates the new Document functions which prepends the prolog an= d > > replaces every instance of ixmlPrintDocument with ixmlPrintNode becau= se > > the two are now no longer interchangeable (except in configure_urlbas= e() > > in the file urlconfig.c, because here you do want a document). > > =20 > you are right, that's the way ixmlPrintDocument should have been=20 > implemented from the beginning. However, I have seen programs using thi= s=20 > existing interface as it is, and manually adding the prolog. > This fix would silently break this interface. To keep backward=20 > compatibility : wouldn't it be preferable to add a new function (e.g.=20 > ixmlPrintDocumentWithProlog or something like that), and document the=20 > deficiency in the old function? >=20 > Also : another user suggests to implement a check in the libupnp, in=20 > order to add the prolog in the device description document if it is=20 > missing : > =20 > http://sourceforge.net/tracker/index.php?func=3Ddetail&aid=3D931724&gro= up_id=3D7189&atid=3D107189 > I don't know if this other patch is correct or not, but we could=20 > implement this idea too, in addition the the ixml fix. >=20 > What's your view ? I'm not much in favor of codifying broken behavior or per user implementation of hacks. I would much prefer to see the library work cleanly out of the box. I don't think the libupnp SDK is in such wide spread use yet that fear of breaking previous usage justifies retaining buggy behavior and thus forcing new users of the library to learn the work-around, surely a time waste for them (as it was for me). I believe a new release could draw attention via release notes and/or a message on the download page about the change in behavior and thus accommodate the earlier adopters. Also, I've cc'ed Jason Vas Dias on this email. Jason sits near me here at Red Hat and it looks like he is going to pick up ownership of this package. Jason tells me there has been some discussion of moving libupnp into core from extras, but I'll leave that discussion to Eric and Jason to pursue. Also, as a side note, I've implemented a few upnp programs using the SDK. I started with the TV samples whose coding style I found a bit awkward and in the process rewrote and reformatted much of the samples. It also includes new utilities to set search criteria, handle command line input, etc. If anyone is interested I'm happy to donate the code. --=20 John Dennis <jd...@re...> Red Hat Inc. |