Revert "curl_xml plugin: Fixed tautological pointer comparison error."

[faxm0dem]

Fixes #931
This reverts commit 0afea60.

[faxm0dem]

As suggested by @mfournier, rather than reverting the patch I fixed the test.

Notes

Sorry if any or all of these seem obvious to the reader:

• The initial patch fixed a clang compiler error: it detected the codepath inside the if was never reached
• The patch 0afea60 introduced a new bug: it unlocked the previously hidden codepath and made it impossible to use Xpath/InstanceFrom if the XPath expression returns more than one result (is_table)
• The proposed change 94ba3c7 fixes the test (tested with our initial config where XPath expression returns more than one result)

Conclusion

I suggest using one of the following solutions:

1. The one proposed in commit 94ba3c7
2. Remove the if ... {} code altogether

The first solution seems to be the best approach, however due to lacking test suite it would be safer to use the second: it would fix the clang error and not change the plugin functionally

[mfournier]

Thanks for taking care of this @faxm0dem !

I grabbed the opportunity to test out this CI system I working on, and these 2 patches together build fine on a range of gcc and clang versions, with -Wall -Werror. So at least the toolchain deities would be satisfied with this :)

I'd favor the 1st option too. As you seem to be familiar with this issue, would you mind sharing some XML and corresponding collectd config snippets, both with and without the xpath table (ie: some way to test the different conditions which would pass through this block of code) ? Thanks !

[faxm0dem]

I'll do that ASAP. Shall I create a directory maybe in src/tests/fixtures/curl_xml/?

[mfournier]

@faxm0dem mmh, I don't think it's a good idea to start adding random bits of input/output/config to the git repo, without the tests that go with them. If you're able to prove this patchset doesn't change the functionnality of the plugin with a couple of xml & config snippets (shared as comments in this PR would be sufficient AFAIC), it would be great!

You're of course welcome to add some actual tests. But I didn't dig into this much yet, so I won't be of much help. Maybe @octo could give some guidance on how this would be done in this case ?

[faxm0dem]

Okay, no problem just thought we could start adding some fixtures there.
Here's what I came up with

[mfournier]

Thanks @faxm0dem, this was very helpful !

I noticed that the examples you provided triggered a couple of resource leaks I corrected in af8f87e. Are you able to test curl_xml with this last patch on other configs than the 2 you provided above ?

Other than that, I tested this patchset on a couple of different distros, with different libxml versions, built both with clang and gcc with -Wall -Werror, and all went well.

So just waiting for your feedback on af8f87e before merging this branch.

[faxm0dem]

Unfortunately although slightly stripped down this is the only use-case I h̶a̶v̶e̶had.

[mfournier]

Merged to the oldest release branch as 5e9542b, 6ad8d0d and c6da31f. Many thanks @faxm0dem !!