<br><br><div class="gmail_quote">On Mon, Jan 11, 2010 at 9:26 PM, Florian Forster <span dir="ltr"><<a href="mailto:octo@verplant.org" target="_blank">octo@verplant.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi Amit,<br>
<br>
thank you very much for your patch :)<br></blockquote><div><br>Thanks for applying the patch :)<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
I did the following changes:<br>
<br>
* Re-order all of the functions to get rid of the forward declarations.<br>
* Replace the string and boolean handling functions for config options<br>
with functions declared in src/configfile.h. This way the<br>
functionality is not duplicated.<br>
* Put the type_instance setting and value parsing into a separate<br>
functions. This makes “cx_submit_xpath_values” considerably bit<br>
shorter.<br>
<br>
Please let me know if my changes broke anything – unfortunately I didn't<br>
have the possibility to test them :/<br></blockquote><div><br>I did a sanity testing and the plugin seems to be working fine. <br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
> #define CX_KEY_MAGIC 0x43484b59UL /* CHKY */<br>
> #define CX_IS_KEY(key) (key)->magic == CX_KEY_MAGIC<br>
<br>
What do we need this hack for? I've found only one place where<br>
“c_avl_insert” is called, so the elements in the tree should all be of<br>
the same type, right? I'd be great if you could remove or document this.<br></blockquote><div><br>There is no need of this magic key. It just happens to be there since I borrowed the code from curl_json :). I will clean this up. <br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
Maybe the entire AVL tree should be removed: As far as I see it is only<br>
used to iterate over the values (rather than searching for a specific<br>
key), so a linked list is probably more appropriate.<br></blockquote><div><br>Yeah I agree. I have replaced avl tree with a linked list.<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
Both, “cx_check_type” and “cx_submit_xpath_values” call “plugin_get_ds”.<br>
I think one call could be optimized away.<br></blockquote><div>This is also done.<br><br>Do find the updated patch attached.<br><br>Regards<br>Amit <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
Best regards,<br>
—octo<br>
<br>
[0] <<a href="http://git.verplant.org/?p=collectd.git;a=blob_plain;f=src/curl_xml.c;hb=refs/heads/ag/curl_xml" target="_blank">http://git.verplant.org/?p=collectd.git;a=blob_plain;f=src/curl_xml.c;hb=refs/heads/ag/curl_xml</a>><br>
<font color="#888888">--<br>
</font><div><div></div><div>Florian octo Forster<br>
Hacker in training<br>
GnuPG: 0x91523C3D<br>
<a href="http://verplant.org/" target="_blank">http://verplant.org/</a><br>
</div></div><br>-----BEGIN PGP SIGNATURE-----<br>
Version: GnuPG v1.4.6 (GNU/Linux)<br>
<br>
iD8DBQFLS0olHdggu3Q05IYRAuMXAJ440LTtkMUZaDb7MUq4EdnXw8SXTgCfS3gS<br>
Pat48nthKsGvE3YMI6rIOh0=<br>
=VgpJ<br>
-----END PGP SIGNATURE-----<br>
<br></blockquote></div><br>