Libguestfs - Jul 2019 - Re: [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

On Mon, Jul 08, 2019 at 11:20:44AM +0200, Martin Kletzander
wrote:
> 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 =)
(See below)
> >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.
This is the way we structure other bindings, eg in the Python bindings
python/handle.c contains non-generated methods such as nbd_create and
nbd_close, while the generated methods are placed in python/methods.c.

I don't know how easy that is to achieve in Rust.  Some programming
languages make it really hard to divide implementations across files
(hello Perl).
> 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.
I'm a bit unclear what "strings" (above) and
"substrings" means in
this context.  Do you mean actual string constants?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Mon, Jul 08, 2019 at 10:36:30AM +0100, Richard W.M. Jones
wrote:
>On Mon, Jul 08, 2019 at 11:20:44AM +0200, Martin Kletzander wrote:
>> 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 =)
>
>(See below)
>
>> >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.
>
>This is the way we structure other bindings, eg in the Python bindings
>python/handle.c contains non-generated methods such as nbd_create and
>nbd_close, while the generated methods are placed in python/methods.c.
>
>I don't know how easy that is to achieve in Rust.  Some programming
>languages make it really hard to divide implementations across files
>(hello Perl).
>
>> 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.
>
>I'm a bit unclear what "strings" (above) and
"substrings" means in
>this context.  Do you mean actual string constants?
>
No, I mean the strings printed by the generator, e.g. the one from your example
on how to use a multiline one:


  pr "\
#[allow(unused_imports)]
use std::os::raw::{c_char, c_int, c_uint, c_void};
use std::os::unix::io::RawFd;
[...]
";
>Rich.
>
>-- 
>Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
>Read my programming and virtualization blog: http://rwmj.wordpress.com
>virt-top is 'top' for virtual machines.  Tiny program with many
>powerful monitoring features, net stats, disk stats, logging, etc.
>http://people.redhat.com/~rjones/virt-top