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