Libguestfs - Aug 2014 - Re: Segmentation fault when trying to add binding

Hi,

I am trying to add xmlXPathRegisterNs() to v2v/xml-c.c but I get a seg fault,
the signature should be:
http://xmlsoft.org/html/libxml-xpathInternals.html#xmlXPathRegisterNs

I think I am wrong in CAMLparam and CAMLlocal...,
Following is the patch:

==================================================================================diff
--git a/v2v/xml-c.c b/v2v/xml-c.c
index 4c9bc77..a917c24 100644
--- a/v2v/xml-c.c
+++ b/v2v/xml-c.c
@@ -141,6 +141,16 @@ v2v_xml_xpath_new_context (value docv)
 }
 
 value
+v2v_xml_xpath_register_ns (value prefix, value uri, value xpathctx)
+{
+  CAMLparam3 (prefix, uri, xpathctx);
+  CAMLlocal1 (retval);
+  retval = xmlXPathRegisterNs (BAD_CAST String_val (prefix), BAD_CAST
String_val (uri), xpathctx);
+
+  CAMLreturn (retval);
+}
+
+value
 v2v_xml_xpath_eval_expression (value xpathctxv, value exprv)
 {
   CAMLparam2 (xpathctxv, exprv);
diff --git a/v2v/xml.ml b/v2v/xml.ml
index 78cb022..2e4d222 100644
--- a/v2v/xml.ml
+++ b/v2v/xml.ml
@@ -31,6 +31,7 @@ type node = doc * node_ptr
 external parse_memory : string -> doc = "v2v_xml_parse_memory"
 external xpath_new_context : doc -> xpathctx =
"v2v_xml_xpath_new_context"
 external xpath_eval_expression : xpathctx -> string -> xpathobj =
"v2v_xml_xpath_eval_expression"
+external xpath_register_ns : string -> string -> xpathctx -> int =
"v2v_xml_xpath_register_ns"
 
 external xpathobj_nr_nodes : xpathobj -> int =
"v2v_xml_xpathobj_nr_nodes"
 external xpathobj_get_node_ptr : xpathobj -> int -> node_ptr =
"v2v_xml_xpathobj_get_node_ptr"
diff --git a/v2v/xml.mli b/v2v/xml.mli
index 38bb9cd..cab4395 100644
--- a/v2v/xml.mli
+++ b/v2v/xml.mli
@@ -29,6 +29,8 @@ val xpath_new_context : doc -> xpathctx
 (** xmlXPathNewContext *)
 val xpath_eval_expression : xpathctx -> string -> xpathobj
 (** xmlXPathEvalExpression *)
+val xpath_register_ns : string -> string -> xpathctx -> int
+(** xmlXPathRegisterNs *)
 
 val xpathobj_nr_nodes : xpathobj -> int
 (** Get the number of nodes in the node set of the xmlXPathObjectPtr. *)
===================================================================================

Hi,

On Tuesday 26 August 2014 09:34:42 Shahar Havivi wrote:
> I am trying to add xmlXPathRegisterNs() to v2v/xml-c.c but I get a seg
> fault, the signature should be:
> http://xmlsoft.org/html/libxml-xpathInternals.html#xmlXPathRegisterNs
> 
> I think I am wrong in CAMLparam and CAMLlocal...,
> Following is the patch:
> 
> =====================================================================>
diff --git a/v2v/xml-c.c b/v2v/xml-c.c
> index 4c9bc77..a917c24 100644
> --- a/v2v/xml-c.c
> +++ b/v2v/xml-c.c
> @@ -141,6 +141,16 @@ v2v_xml_xpath_new_context (value docv)
>  }
> 
>  value
> +v2v_xml_xpath_register_ns (value prefix, value uri, value xpathctx)
> +{
> +  CAMLparam3 (prefix, uri, xpathctx);
> +  CAMLlocal1 (retval);
> +  retval = xmlXPathRegisterNs (BAD_CAST String_val (prefix), BAD_CAST
String_val (uri), xpathctx);
> +
> +  CAMLreturn (retval);
> +}
> +
First of all, you are using the xpathctx parameter directly as
parameter for xmlXPathRegisterNs, while it is an OCaml value which
wraps the actual object; see how it is done in e.g.
v2v_xml_xpath_eval_expression, using the Xpathctx_val macro.

Second, the order of the arguments in your xmlXPathRegisterNs is wrong:
- yours: xmlXPathRegisterNs(prefix, uri, ctxt)
- actual function: xmlXPathRegisterNs(ctxt, prefix, uri)

-- 
Pino Toscano

On Tue, Aug 26, 2014 at 09:34:42AM +0300, Shahar Havivi
wrote:
> Hi,
> 
> I am trying to add xmlXPathRegisterNs() to v2v/xml-c.c but I get a seg
fault,
> the signature should be:
> http://xmlsoft.org/html/libxml-xpathInternals.html#xmlXPathRegisterNs
> 
> I think I am wrong in CAMLparam and CAMLlocal...,
> Following is the patch:
> 
>
==================================================================================>
diff --git a/v2v/xml-c.c b/v2v/xml-c.c
> index 4c9bc77..a917c24 100644
> --- a/v2v/xml-c.c
> +++ b/v2v/xml-c.c
> @@ -141,6 +141,16 @@ v2v_xml_xpath_new_context (value docv)
>  }
>  
>  value
> +v2v_xml_xpath_register_ns (value prefix, value uri, value xpathctx)
> +{
> +  CAMLparam3 (prefix, uri, xpathctx);
> +  CAMLlocal1 (retval);
> +  retval = xmlXPathRegisterNs (BAD_CAST String_val (prefix), BAD_CAST
String_val (uri), xpathctx);
> +
> +  CAMLreturn (retval);
An 'int' isn't a 'value', so this will not work and likely
segfault.
For the representation of 'value' see:

  http://rwmj.wordpress.com/2009/08/04/ocaml-internals/#content
  (and follow up articles).

In any case, here you need to turn the return value into an exception,
ie something like this untested code:

  value
  v2v_xml_xpath_register_ns (value prefix, value uri, value xpathctx)
  {
    CAMLparam3 (prefix, uri, xpathctx);
    int r;

    r = xmlXPathRegisterNs (...);
    if (r == -1)
      caml_invalid_argument ("some suitable error message here ...");

    /* return unit */
    CAMLreturn (Val_unit);
  }
> +external xpath_register_ns : string -> string -> xpathctx -> int
= "v2v_xml_xpath_register_ns"
Needs to return unit, not int.  If the libxml2 function returns -1
then this will throw an exception (Invalid_arg).
> +val xpath_register_ns : string -> string -> xpathctx -> int
> +(** xmlXPathRegisterNs *)
Same here.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Tue, Aug 26, 2014 at 09:10:11AM +0200, Pino Toscano
wrote:
> Hi,
> 
> On Tuesday 26 August 2014 09:34:42 Shahar Havivi wrote:
> > I am trying to add xmlXPathRegisterNs() to v2v/xml-c.c but I get a seg
> > fault, the signature should be:
> > http://xmlsoft.org/html/libxml-xpathInternals.html#xmlXPathRegisterNs
> > 
> > I think I am wrong in CAMLparam and CAMLlocal...,
> > Following is the patch:
> > 
> >
=====================================================================> >
diff --git a/v2v/xml-c.c b/v2v/xml-c.c
> > index 4c9bc77..a917c24 100644
> > --- a/v2v/xml-c.c
> > +++ b/v2v/xml-c.c
> > @@ -141,6 +141,16 @@ v2v_xml_xpath_new_context (value docv)
> >  }
> > 
> >  value
> > +v2v_xml_xpath_register_ns (value prefix, value uri, value xpathctx)
> > +{
> > +  CAMLparam3 (prefix, uri, xpathctx);
> > +  CAMLlocal1 (retval);
> > +  retval = xmlXPathRegisterNs (BAD_CAST String_val (prefix), BAD_CAST
String_val (uri), xpathctx);
> > +
> > +  CAMLreturn (retval);
> > +}
> > +
> 
> First of all, you are using the xpathctx parameter directly as
> parameter for xmlXPathRegisterNs, while it is an OCaml value which
> wraps the actual object; see how it is done in e.g.
> v2v_xml_xpath_eval_expression, using the Xpathctx_val macro.
> 
> Second, the order of the arguments in your xmlXPathRegisterNs is wrong:
> - yours: xmlXPathRegisterNs(prefix, uri, ctxt)
> - actual function: xmlXPathRegisterNs(ctxt, prefix, uri)
This too.

Notice that Val_foo() means convert foo to value, and
Foo_val() means convert value to foo.

In that file, only node_ptr/xmlNodePtr can be cast directly from value
to the C type, and that's for a very special and peculiar reason.  For
everything else you have to convert between value and the C type (and
vice versa) using the appropriate macro.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v