metadata: guard all element-content derefs with node_text() helper

[bornmw]

Summary

Fixes #126 — wcm segfaults at startup when a loaded plugin's metadata contains a metadata element with empty content (<value></value>, <_short></_short>, etc.).

Root cause

A metadata element with no text content leaves node->children == NULL in libxml2, so reading node->children->content is a null deref → SIGSEGV.

The get_plugin_data / Option constructors already guarded against this for <min>, <max>, <precision>, <hint>, <default> and <category> via if (!node->children) continue; — but the guard was missing for several other element types:

• Option::_short, Option::_long
• Option::type (dynamic-list)
• Option::desc child loop (<value> / <_name>, the crash from wcm segfaults at startup when a loaded plugin's metadata has an empty <value> element in an option <desc> #126)
• Plugin::_short, Plugin::_long
• Group::_short, Subgroup::_short

Change

Add a small helper that centralises the "is it empty?" logic:

static xmlChar *
node_text(const xmlNode *node)
{
    return (node->children) ? node->children->content : nullptr;
}

and use it at every site, with a null-check before use. The previously-guarded sites are untouched; the previously-unguarded ones now call node_text() and continue when it returns NULL.

This is option 1 from the discussion: a DRY helper rather than a blanket top-of-loop guard (which would change semantics for <default></default>, where an empty string is a valid distinct value).

Testing

• No unit-test framework exists in wcm; CI is build-on-Alpine + uncrustify style check. The change is a simple helper + identical guard/node_text substitutions, so it is style-clean.
• Reproduced the crash with the metadata from the issue before the fix; after the fix wcm starts normally with that metadata.
• Verified all remaining node->children->content sites in the file are within an existing if (!node->children) guard.

[soreau]

NAK. The metadata should be fixed, not consumers of the metadata. It does not make sense to have an empty <element> in the first place. Thank you for your interest. Closes #126.

[bornmw]

NAK. The metadata should be fixed, not consumers of the metadata. It does not make sense to have an empty <element> in the first place. Thank you for your interest. Closes #126.

There are already checks for empty '<element>' in the code, e.g. <min>/<max>/<precision>/<hint>/<category>
So it does make sense to have empty elements
Same case with the <default> - it already treats empty content as a valid value, and does not segfault
Why would you guard some fields and let others segfault?

[soreau]

I use wcm with all upstream metadata and it does not crash. Can you show an example of the offending metadata you are using to crash it?

[bornmw]

I use wcm with all upstream metadata and it does not crash. Can you show an example of the offending metadata you are using to crash it?

I have my own plugins that are not mainstream.
I understand that the value should have something that is not empty, but segfault is definitely not the way to enforce that
It should be something like what default does - a WARN or an ERROR, but segfault?

[soreau]

It should be something like what default does - a WARN or an ERROR, but segfault?

I would accept a PR that prints a warning and exits in these cases.