This code:
char *printMissingTag(Tag tag)
{
char *s = translateXMLEnumName[tag];
int len = strlen(s)+14+20;
char *r = malloc(len);
snprintf(r, len, "%s Missing tag: %s %s",RED, s, RESET);
return r;
}
Is likely to be incorrect, because it uses hard-coded numbers and it took me a
good while to figure out where the 14 and 20 were derived from. Getting this
wrong can lead to buffer overflows, which can result in crashes or security
vulnerabilities or other problems.
There’s a function called DFFormatString in DFString.h which will basically do
the above for you. So you can just use the following instead:
char *printMissingTag(Tag tag)
{
char *s = translateXMLEnumName[tag];
return DFFormatString("%s Missing tag: %s %s",RED, s, RESET);
}
Also, the fact that you’re assigning this to the value attribute of a DFNode
object will lead to a memory leak, because you’re not freeing that memory
anywhere. Actually all of the memory for DFNode objects and the attributes and
string values they maintained is supposed to be allocated by the DFDocument’s
own internal memory allocator, which, when you release the last reference to a
DFDocument object, releases all the memory in one go.
There’s a function called DFSetNodeValue which should always be used to set the
value of text nodes. You can see the implementation of this in DFDOM.c; it uses
the document’s memory allocator to make a copy of the string, and then assign
that to the node’s value. So if you were to keep the printMissingTag as it
exists above, you would replace the following code in ODFText.c:
child->value = printMissingTag(odfChild->tag);
with:
char *value = printMissingTag(odfChild->tag);
DFSetNodeValue(child,value);
free(value);
Note that we free the result of printMissingTag here, as otherwise the memory
will leak.
I’d also suggest a different name than “printMissingTag”, since the function
itself doesn’t actually do any printing (it instead returns a string, which the
caller can either print or do something else with, e.g. assigning it to a text
node value).
Finally, while the following line:
child = DFCreateChildElement(htmlNode, 0);
creates an invalid node; you’re using 0 as the tag. Actually you’re creating a
text node here, not an element, so DFCreateChildTextNode would be the
appropriate function to use here instead. Combining this with the above, you
could replace the following two lines:
child = DFCreateChildElement(htmlNode, 0);
child->value = printMissingTag(odfChild->tag);
with:
char *value = printMissingTag(odfChild->tag);
child = DFCreateChildTextNode(htmlNode,value);
free(value);
—
Dr Peter M. Kelly
[email protected]
PGP key: http://www.kellypmk.net/pgp-key <http://www.kellypmk.net/pgp-key>
(fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)
> On 10 May 2015, at 8:52 am, Gabriela Gibson <[email protected]> wrote:
>
> Hi,
>
> So far I got my branch to produce a list of html nodes (and report on
> still missing stuff).
>
> This is probably a good point to have a look if the approach I'm using
> here is any good.
>
> It of course has quite a few warts still, and I think I will need to
> add function pointers to the ODF_to_HTML_key struct to deal with some
> special cases. If that struct is a good idea that is.
>
> The branch can be found here:
>
> https://github.com/apache/incubator-corinthia/commit/c81e68626489b9515e7e8f3a5ce5d38ac8f59af0
>
> I added the test odt file I was using, plus the current output of the program.
>
> thanks for looking,
>
> G
>
> --
> Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
