[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [libnbd PATCH] RFC: Add bindings for Rust language



On Mon, Jul 08, 2019 at 09:58:20AM +0100, Richard W.M. Jones wrote:
The patch seems OK in general.


The libnbd-sys part is actually complete.  Well, except the build script, but
that one is not completely necessary.  And the documentation.

The wrappers are missing quite a few things to be usable, but it should not be
*that* difficult once someone wants to play with it.

What bothers me a lot is the way I am composing some of the strings.  The only
way why I shamelessly sent this was that the code runs once to generate the file
and it is not part of the resulting product =)

On Sun, Jul 07, 2019 at 11:39:29PM +0200, Martin Kletzander wrote:
The way the code is generated is also not nice, I wish there was
more code actually written in some files and not generated by the
generator (as much hard-coded static strings as possible), maybe
similarly to the states.c, I don't know.

Can you expand on what you mean by this?


For example the `impl` blocks can be combined from different modules/files, so
the implementations that are just constant (not generated dynamically, just
static strings printed out) can be in a separate file instead of being written
every time by the generator.

Maybe some substrings could be identified to be common and then copied from a
file instead of duplicating the data.  But this is not needed.

Also up for discussion is whether the libnbd crate should be
separate since the higher-level functionality it should provide will
not be tightly coupled with libnbd itself and the releases
(especially the numbers) do not need to happen in sync.

I also didn't understand what this means.

It's a matter of personal preference but you can use multi-line string
constants in OCaml which can be unlimited in length, so this:

+  pr "#[allow(unused_imports)]\n";
+  pr "use std::os::raw::{c_char, c_int, c_uint, c_void};\n";
+  pr "use std::os::unix::io::RawFd;\n";
+  pr "use std::ffi::CStr;\n";
+  pr "use libnbd_sys::*;\n";
+  pr "use libc;\n";
+  pr "\n";
(etc)

can be written as:

 pr "\
#[allow(unused_imports)]
use std::os::raw::{c_char, c_int, c_uint, c_void};
use std::os::unix::io::RawFd;
[...]
";

This makes it much nicer, I noticed it in the generator, but I have not spent
any time figuring out why my editor complained about it.  But I wasted a lot of
time on missing/extra semicolons, so it could've been something silly like that.

I'll spend some time one it (that is if I find some spare free time in near
future).


These are still C-like printf-like strings so you still need to escape
%, " and \.

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

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]