On Wed, Aug 16, 2006 at 07:54:49AM -0400, Rob Richards wrote:
> Daniel Veillard wrote:
> >On Tue, Aug 15, 2006 at 07:06:48PM -0400, Rob Richards wrote:
> >
> >>When using xmlwriter attributes , document encoding that has been set is
> >>not passed to xmlAttrSerializeTxtContent, so character references are
> >>written when needed.
> >>The xmlDocPtr passed to xmlAttrSerializeTxtContent is only used to check
> >>encoding, so I was wondering if the code in the attached patch is fine
> >>to fix this (it fakes a document so any changes within
> >>xmlAttrSerializeTxtContent to manipulate the passed document could
> >>possibly blow up - though i dont see any need/reason to ever do this).
> >>
> >
> > Hum, that's a bit nasty, because none of the fields in the document
> >structure are initialized (except encoding), so it internally use an
> >API based not on the API description but on its implementation. If at
> >some point someone add something like entity lookup to
> >xmlAttrSerializeTxtContent or just check the document type, suddently
> >the writer would crash in an unexpected way.
> > So fields should at least be zero initialized, which is probably
> >a bit costly if done for every single attribute emitted. So while passing
> >a document might be a good idea, it's better to create and initialize that
> >document when getting the xmlTextWriterPtr. Maybe adding a doc in
> >struct _xmlTextWriter and making sure it's at least minimally initialized
> >and properly freed with the xmlTextWriter is a good idea. That sounds
> >far more clean to me, but a bit more work indeed :-)
> >
> >Daniel
> >
>
> Was tying to keep the change as minimal as possible, so how about this one.
> A doc has been added to the struct and is always created within
> xmlNewTextWriter(). This would at least provide a doc that could be used
> for this type of thing, rather than checking if one exists and having to
> create one if needed at different points within the xmlwriter API. This
> doc is also completely independent of any doc that may exist within a
> context.
> ***************
> *** 539,544 ****
> --- 546,553 ----
> if (encoder != NULL) {
> writer->out->conv = xmlBufferCreateSize(4000);
> xmlCharEncOutFunc(encoder, writer->out->conv, NULL);
> + if (writer->doc)
> + writer->doc->encoding = xmlStrdup((xmlChar
> *)writer->out->encoder->name);
> } else
> writer->out->conv = NULL;
I would suggest to guard that a bit better as
if ((writer->doc != NULL) && (writer->doc->encoding == NULL))
just in case writer->doc gets inherited in some way in the future or if
a default encoding shows up.
Otherwise it looks fine to me, just run the regression tests for good measure
before commiting, one never knows :-)
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
[EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
_______________________________________________
xml mailing list, project page http://xmlsoft.org/
[email protected]
http://mail.gnome.org/mailman/listinfo/xml
