On Fri, 17 Mar 2023 at 18:01, Jim Jones <jim.jo...@uni-muenster.de> wrote: > > After some more testing I realized that v5 was leaking the xmlDocPtr. > > Now fixed in v6.
Few comments: 1) Why the default option was chosen without comments shouldn't it be the other way round? +opt_xml_serialize_format: + INDENT { $$ = XMLSERIALIZE_INDENT; } + | NO INDENT { $$ = XMLSERIALIZE_NO_FORMAT; } + | CANONICAL { $$ = XMLSERIALIZE_CANONICAL; } + | CANONICAL WITH NO COMMENTS { $$ = XMLSERIALIZE_CANONICAL; } + | CANONICAL WITH COMMENTS { $$ = XMLSERIALIZE_CANONICAL_WITH_COMMENTS; } + | /*EMPTY*/ { $$ = XMLSERIALIZE_NO_FORMAT; } 2) This should be added to typedefs.list: +typedef enum XmlSerializeFormat +{ + XMLSERIALIZE_INDENT, /* pretty-printed xml serialization */ + XMLSERIALIZE_CANONICAL, /* canonical form without xml comments */ + XMLSERIALIZE_CANONICAL_WITH_COMMENTS, /* canonical form with xml comments */ + XMLSERIALIZE_NO_FORMAT /* unformatted xml representation */ +} XmlSerializeFormat; 3) This change is not required: return result; + #else NO_XML_SUPPORT(); return NULL; 4) This comment body needs slight reformatting: + /* + * Parse the input according to the xmloption. + * XML canonical expects a well-formed XML input, so here in case of + * XMLSERIALIZE_CANONICAL or XMLSERIALIZE_CANONICAL_WITH_COMMENTS we + * force xml_parse() to parse 'data' as XMLOPTION_DOCUMENT despite + * of the XmlOptionType given in 'xmloption_arg'. This enables the + * canonicalization of CONTENT fragments if they contain a singly-rooted + * XML - xml_parse() will thrown an error otherwise. + */ 5) Similarly here too: - if (newline == NULL || xmlerrcxt->err_occurred) + * Emit declaration only if the input had one. Note: some versions of + * xmlSaveToBuffer leak memory if a non-null encoding argument is + * passed, so don't do that. We don't want any encoding conversion + * anyway. + */ + if (decl_len == 0) 6) Similarly here too: + /* + * Deal with the case where we have non-singly-rooted XML. + * libxml's dump functions don't work well for that without help. + * We build a fake root node that serves as a container for the + * content nodes, and then iterate over the nodes. + */ 7) Similarly here too: + /* + * We use this node to insert newlines in the dump. Note: in at + * least some libxml versions, xmlNewDocText would not attach the + * node to the document even if we passed it. Therefore, manage + * freeing of this node manually, and pass NULL here to make sure + * there's not a dangling link. + */ 8) Should this: + * of the XmlOptionType given in 'xmloption_arg'. This enables the + * canonicalization of CONTENT fragments if they contain a singly-rooted + * XML - xml_parse() will thrown an error otherwise. Be: + * of the XmlOptionType given in 'xmloption_arg'. This enables the + * canonicalization of CONTENT fragments if they contain a singly-rooted + * XML - xml_parse() will throw an error otherwise. Regards, Vignesh