2:41 a.m.
In v1 of this patch, there was the (small) possibility that 'g' might
have been garbage collected while we were in the C function.
Avoid this by passing 'g' to the function as well as the C pointer, so
that 'g' is pinned as a garbage collector root [by CAMLparam5] so it
cannot be collected while we're in the function.
Rich.
2:41 a.m.
We can now use the Guestfs.c_pointer method to access the underlying
guestfs_h *. So no need to use internal APIs for this.
---
customize/perl_edit-c.c | 18 ++++++------------
customize/perl_edit.ml | 10 ++++++++--
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/customize/perl_edit-c.c b/customize/perl_edit-c.c
index 753d990..7191bef 100644
--- a/customize/perl_edit-c.c
+++ b/customize/perl_edit-c.c
@@ -25,30 +25,24 @@
#include <caml/alloc.h>
#include <caml/memory.h>
#include <caml/mlvalues.h>
+#include <caml/fail.h>
#include "file-edit.h"
-/**
- * We try to reuse the internals of the OCaml binding (for extracting
- * the guestfs handle, and raising errors); hopefully this should be safe,
- * as long as it's kept internal within the libguestfs sources.
- */
-#include "../ocaml/guestfs-c.h"
-
#pragma GCC diagnostic ignored "-Wmissing-prototypes"
value
-virt_customize_edit_file_perl (value verbosev, value gv, value filev,
- value exprv)
+virt_customize_edit_file_perl (value verbosev, value gv, value gpv,
+ value filev, value exprv)
{
- CAMLparam4 (verbosev, gv, filev, exprv);
+ CAMLparam5 (verbosev, gv, gpv, filev, exprv);
int r;
- guestfs_h *g = Guestfs_val (gv);
+ guestfs_h *g = (guestfs_h *) Int64_val (gpv);
r = edit_file_perl (g, String_val (filev), String_val (exprv), NULL,
Bool_val (verbosev));
if (r == -1)
- guestfs_int_ocaml_raise_error (g, "edit_file_perl");
+ caml_failwith (guestfs_last_error (g) ? : "edit_file_perl: unknown
error");
CAMLreturn (Val_unit);
}
diff --git a/customize/perl_edit.ml b/customize/perl_edit.ml
index f1f06cc..5cd250b 100644
--- a/customize/perl_edit.ml
+++ b/customize/perl_edit.ml
@@ -18,6 +18,12 @@
open Common_utils
-external c_edit_file : verbose:bool -> Guestfs.t -> string -> string -> unit
+external c_edit_file : verbose:bool -> Guestfs.t -> int64 -> string -> string
-> unit
= "virt_customize_edit_file_perl"
-let edit_file g file expr = c_edit_file (verbose ()) g file expr
+let edit_file g file expr =
+ (* Note we pass original 'g' even though it is not used by the
+ * callee. This is so that 'g' is kept as a root on the stack, and
+ * so cannot be garbage collected while we are in the c_edit_file
+ * function.
+ *)
+ c_edit_file (verbose ()) g (Guestfs.c_pointer g) file expr
--
2.7.4
3:30 a.m.
On Tuesday 12 April 2016 08:41:37 Richard W.M. Jones wrote:
We can now use the Guestfs.c_pointer method to access the underlying
guestfs_h *. So no need to use internal APIs for this.
---
LGTM.
Did it cause any particular issue? (Not that it is a problem, just to
know whether it was doing anything problematic.)
Thanks,
--
Pino Toscano
4:35 a.m.
On Tue, Apr 12, 2016 at 10:30:11AM +0200, Pino Toscano wrote:
On Tuesday 12 April 2016 08:41:37 Richard W.M. Jones wrote:
> We can now use the Guestfs.c_pointer method to access the underlying
> guestfs_h *. So no need to use internal APIs for this.
> ---
LGTM.
Did it cause any particular issue? (Not that it is a problem, just to
know whether it was doing anything problematic.)
No, there was no particular problem. I just happened to be looking at
that code because it contained a '/**' comment, and I'm doing
something connected to that (coming soon).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
8:33 a.m.
On Tuesday 12 April 2016 08:41:37 Richard W.M. Jones wrote:
We can now use the Guestfs.c_pointer method to access the underlying
guestfs_h *. So no need to use internal APIs for this.
---
customize/perl_edit-c.c | 18 ++++++------------
customize/perl_edit.ml | 10 ++++++++--
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/customize/perl_edit-c.c b/customize/perl_edit-c.c
index 753d990..7191bef 100644
--- a/customize/perl_edit-c.c
+++ b/customize/perl_edit-c.c
@@ -25,30 +25,24 @@
#include <caml/alloc.h>
#include <caml/memory.h>
#include <caml/mlvalues.h>
+#include <caml/fail.h>
#include "file-edit.h"
-/**
- * We try to reuse the internals of the OCaml binding (for extracting
- * the guestfs handle, and raising errors); hopefully this should be safe,
- * as long as it's kept internal within the libguestfs sources.
- */
-#include "../ocaml/guestfs-c.h"
-
#pragma GCC diagnostic ignored "-Wmissing-prototypes"
value
-virt_customize_edit_file_perl (value verbosev, value gv, value filev,
- value exprv)
+virt_customize_edit_file_perl (value verbosev, value gv, value gpv,
+ value filev, value exprv)
{
- CAMLparam4 (verbosev, gv, filev, exprv);
+ CAMLparam5 (verbosev, gv, gpv, filev, exprv);
int r;
- guestfs_h *g = Guestfs_val (gv);
+ guestfs_h *g = (guestfs_h *) Int64_val (gpv);
This causes warnings on 32bit platforms though:
perl_edit-c.c: In function 'virt_customize_edit_file_perl':
perl_edit-c.c:40:18: error: cast to pointer from integer of different size
[-Werror=int-to-pointer-cast]
guestfs_h *g = (guestfs_h *) Int64_val (gpv);
^
What about the following patch?
Subject: [PATCH] customize: cast value to intptr_t for pointer usage
Cast a Int64 OCaml value to intptr_t before casting it to a pointer, so
this avoids warnings about casting an int to pointer.
Fixes commit 80a13b7216340b2bee437c39db2375b61804e133.
---
customize/perl_edit-c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/customize/perl_edit-c.c b/customize/perl_edit-c.c
index 7191bef..9558d07 100644
--- a/customize/perl_edit-c.c
+++ b/customize/perl_edit-c.c
@@ -37,7 +37,7 @@ virt_customize_edit_file_perl (value verbosev, value gv, value gpv,
{
CAMLparam5 (verbosev, gv, gpv, filev, exprv);
int r;
- guestfs_h *g = (guestfs_h *) Int64_val (gpv);
+ guestfs_h *g = (guestfs_h *) (intptr_t) Int64_val (gpv);
r = edit_file_perl (g, String_val (filev), String_val (exprv), NULL,
Bool_val (verbosev));
Thanks,
--
Pino Toscano
8:58 a.m.
On Thu, Apr 14, 2016 at 03:33:25PM +0200, Pino Toscano wrote:
On Tuesday 12 April 2016 08:41:37 Richard W.M. Jones wrote:
> We can now use the Guestfs.c_pointer method to access the underlying
> guestfs_h *. So no need to use internal APIs for this.
> ---
> customize/perl_edit-c.c | 18 ++++++------------
> customize/perl_edit.ml | 10 ++++++++--
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/customize/perl_edit-c.c b/customize/perl_edit-c.c
> index 753d990..7191bef 100644
> --- a/customize/perl_edit-c.c
> +++ b/customize/perl_edit-c.c
> @@ -25,30 +25,24 @@
> #include <caml/alloc.h>
> #include <caml/memory.h>
> #include <caml/mlvalues.h>
> +#include <caml/fail.h>
>
> #include "file-edit.h"
>
> -/**
> - * We try to reuse the internals of the OCaml binding (for extracting
> - * the guestfs handle, and raising errors); hopefully this should be safe,
> - * as long as it's kept internal within the libguestfs sources.
> - */
> -#include "../ocaml/guestfs-c.h"
> -
> #pragma GCC diagnostic ignored "-Wmissing-prototypes"
>
> value
> -virt_customize_edit_file_perl (value verbosev, value gv, value filev,
> - value exprv)
> +virt_customize_edit_file_perl (value verbosev, value gv, value gpv,
> + value filev, value exprv)
> {
> - CAMLparam4 (verbosev, gv, filev, exprv);
> + CAMLparam5 (verbosev, gv, gpv, filev, exprv);
> int r;
> - guestfs_h *g = Guestfs_val (gv);
> + guestfs_h *g = (guestfs_h *) Int64_val (gpv);
This causes warnings on 32bit platforms though:
perl_edit-c.c: In function 'virt_customize_edit_file_perl':
perl_edit-c.c:40:18: error: cast to pointer from integer of different size
[-Werror=int-to-pointer-cast]
guestfs_h *g = (guestfs_h *) Int64_val (gpv);
^
What about the following patch?
Subject: [PATCH] customize: cast value to intptr_t for pointer usage
Cast a Int64 OCaml value to intptr_t before casting it to a pointer, so
this avoids warnings about casting an int to pointer.
Fixes commit 80a13b7216340b2bee437c39db2375b61804e133.
---
customize/perl_edit-c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/customize/perl_edit-c.c b/customize/perl_edit-c.c
index 7191bef..9558d07 100644
--- a/customize/perl_edit-c.c
+++ b/customize/perl_edit-c.c
@@ -37,7 +37,7 @@ virt_customize_edit_file_perl (value verbosev, value gv, value gpv,
{
CAMLparam5 (verbosev, gv, gpv, filev, exprv);
int r;
- guestfs_h *g = (guestfs_h *) Int64_val (gpv);
+ guestfs_h *g = (guestfs_h *) (intptr_t) Int64_val (gpv);
r = edit_file_perl (g, String_val (filev), String_val (exprv), NULL,
Bool_val (verbosev));
ACK.
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/
3145
days inactive
3147
days old
5 comments
2 participants
participants (2)
-
Pino Toscano -
Richard W.M. Jones