#121 Fix possible use of undefined variable

[Daniel Macks]

Building xmlstarlet-1.6.1 on OS X 10.13:

src/xml_edit.c:361:18: warning: variable 'node' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
        else if (type == XML_TEXT)
                 ^~~~~~~~~~~~~~~~
src/xml_edit.c:371:48: note: uninitialized use occurs here
        xmlXPathNodeSetAdd(previous_insertion, node);
                                               ^~~~

Sure enough, the declaration in line 339 is:

       xmlNodePtr node;

not all XmlNodeType enum values of the type variabe are handled by the if/else-if construct (and no "else" fallback) prior to xmlXPathNodeSetAdd. First, this chunk of code might be clearer as a switch/case, as attached patch.

This change makes warning even clearer:

~~~
src/xml_edit.c:347:17: warning: 4 enumeration values not handled in switch:
'XML_UNDEFINED', 'XML_COMT', 'XML_CDATA'... [-Wswitch]
switch (type)
^
src/xml_edit.c:347:17: note: add missing switch cases
switch (type)
^
~~~

So there needs to be an "else" for the original code or a "default" for the switch/case implementation. When an unhandled type is passed, should it set node=NULL (either as an "else" or "default" block, or as an initial value in the declaration? Or should there be a runtime warning or error?

[Daniel Macks]

I guess this is both a patch and a bug-report...depending on whether patch is accepted and how upstream thinks the actual bug should be handled would alter what patch to fix it. Shame I didn't think this all through before being forced to choose which tracker to post.

[Noam Postavsky]

Ticket moved from /p/xmlstar/patches/20/

[Noam Postavsky]

I think it shouldn't be possible to get the other node types in that place (although it's been a while since I've looked at this code, so I could well be wrong about this), so probably we should signal an error if we get them.

I moved this ticket to the Bugs section, because the patch is just a supplement to the bug report.