On Thu, Mar 17, 2022 at 03:33:32PM +0100, Laszlo Ersek wrote: > On 03/15/22 11:31, Richard W.M. Jones wrote: > > + rpmVSFlags oflags; > > + > > CAMLparam1 (unitv); > > ts = rpmtsCreate (); > > + > > + /* Disable signature checking (RHBZ#2064182). */ > > + oflags = rpmtsVSFlags (ts); > > + rpmtsSetVSFlags (ts, oflags | RPMVSF_MASK_NOSIGNATURES); > > + > > iter = rpmtsInitIterator (ts, RPMDBI_PACKAGES, NULL, 0); > > CAMLreturn (Val_unit); > > } > > > > The logic seems OK, but the execution seems to conflict with the > *letter* of "Interfacing C with OCaml": > > https://ocaml.org/manual/intfc.html#ss:c-simple-gc-harmony > > """ > Rule 1 A function that has parameters or local variables of type value > must begin with a call to one of the CAMLparam macros [...] > """ > > All ocaml-interfacing C functions I've seen thus far in the v2v > projects, and one function that I just checked (namely > guestfs_int_daemon_rpm_next_application()), conform to this. > > The documentation in "/usr/lib64/ocaml/caml/memory.h" seems to support > this requirement: > > """ > /* The following macros are used to declare C local variables and > function parameters of type [value]. > > The function body must start with one of the [CAMLparam] macros. > """ > > So, even if it may not matter in practice, I suggest introducing > "oflags" *after* CAMLparam1().
In fact as -- erm -- pushed I removed the local variable, so I guess we're all good. (This was an urgent fix for RHEL 9.0 because openssl changes there broke everything.) The CAMLparam macro creates a linked list of stack frames for storing the roots, and I think our use of a local variable which isn't a "value" would have been OK, but definitely better to follow the letter of the documentation in case the macros change in future. > With that update: > > Acked-by: Laszlo Ersek <[email protected]> Thanks for the review, 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/ _______________________________________________ Libguestfs mailing list [email protected] https://listman.redhat.com/mailman/listinfo/libguestfs
