[Libguestfs] [PATCH v5 03/10] mllib: ocaml wrapper for lib/osinfo

Pino Toscano ptoscano at redhat.com
Tue Apr 4 17:04:23 UTC 2017


On Thursday, 23 March 2017 10:02:41 CEST Cédric Bosdonnat wrote:
> Provide osinfo database parsing API in OCAML.

s/OCAML/OCaml/

> diff --git a/lib/osinfo.c b/lib/osinfo.c
> index 5ccb554be..083872669 100644
> --- a/lib/osinfo.c
> +++ b/lib/osinfo.c
> @@ -52,6 +52,21 @@
>  
>  #include "osinfo.h"
>  
> +#ifndef GUESTFS_PRIVATE
> +#undef perrorf
> +#define perrorf(g,...)                          \
> +{                                               \
> +  CLEANUP_FREE char *msg = NULL;                \
> +  ignore_value (asprintf (&msg, __VA_ARGS__));  \
> +  perror (msg);                                 \
> +  /* Ignoring the result is fine since perror   \
> +   * can take NULL input */                     \
> +}
> +
> +#undef debug
> +#define debug(g,...) fprintf (stderr, __VA_ARGS__)
> +#endif /* GUESTFS_PRIVATE */

Please make perror a proper inline function, or use the pattern
  do { ... } while (0)

Also, messages in perror/debug do not have newlines, since the
debugging mechanism will print them: thus these local stubs need to
print newlines at the end of messages, otherwise the output is awkward:

osinfo short ID (can be found with osinfo-query os command): fedora25
osinfo: loading 3-level-directories database from /usr/share/osinfo/osExpandable partition: 

> diff --git a/mllib/osinfo-c.c b/mllib/osinfo-c.c
> new file mode 100644
> index 000000000..393208244
> --- /dev/null
> +++ b/mllib/osinfo-c.c
> @@ -0,0 +1,102 @@
> +/* Bindings for osinfo db reading function.
> + * Copyright (C) 2017 SUSE Inc.

Considering these bindings are mostly based on the Visit one, I'd say
that that copyright notice should be retained here.

LGTM otherwise.

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170404/b9e5bc0b/attachment.sig>


More information about the Libguestfs mailing list