Laszlo Ersek
2023-Apr-21 07:34 UTC
[Libguestfs] [libnbd PATCH v2 0/4] start wrapping generated C source code harder at 80 chars
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
v1: https://listman.redhat.com/archives/libguestfs/2023-April/031340.html
Patches #1 and #2 are unchanged (beyond picking up Rich's R-b). Patch#3
is new. I've updated patch#4 (see the notes on the patch).
Thanks for reviewing,
Laszlo
Laszlo Ersek (4):
scripts/git.orderfile: drop "generator/generator"
scripts/git.orderfile: put *.mli first among generator files
generator/utils: import the "spaces" func from libguestfs-common
Std_utils
generator: indent C argument list 2 spaces relative to function
designator
generator/C.ml | 25 +++++++++++++++-----
generator/C.mli | 3 ++-
generator/GoLang.ml | 4 ++--
generator/utils.ml | 5 ++--
generator/utils.mli | 1 +
scripts/git.orderfile | 2 +-
6 files changed, 28 insertions(+), 12 deletions(-)
base-commit: 86820dbab49723d8ce3182fb0b94f3006f151b57
Laszlo Ersek
2023-Apr-21 07:34 UTC
[Libguestfs] [libnbd PATCH v2 1/4] scripts/git.orderfile: drop "generator/generator"
Since commit 33f6d55891f2 ("generator: Split the generator script into
separate files and compile it.", 2020-02-29),
"generator/generator" has
been a binary file and has not been tracked under version control; the
commit even added "generator/generator" to ".gitignore".
Therefore, we
don't expect "generator/generator" to show up in any patches; drop
it from
"scripts/git.orderfile".
Updates: 33f6d55891f2c00a5deac190954ea6a93a3e6894
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
Notes:
v2:
- pick up Rich's R-b
scripts/git.orderfile | 1 -
1 file changed, 1 deletion(-)
diff --git a/scripts/git.orderfile b/scripts/git.orderfile
index 27bd446195e6..6f214f4aca7e 100644
--- a/scripts/git.orderfile
+++ b/scripts/git.orderfile
@@ -38,7 +38,6 @@ common/*.h
common/*/*.h
# Generator files.
-generator/generator
generator/*
# Source files.
Laszlo Ersek
2023-Apr-21 07:34 UTC
[Libguestfs] [libnbd PATCH v2 2/4] scripts/git.orderfile: put *.mli first among generator files
Similarly to how we put *.h (i.e., interface) files first among C-language
source code, move *.mli to the top of the (OCaml-language) generator
files in patches.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
Notes:
v2:
- pick up Rich's R-b
scripts/git.orderfile | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/git.orderfile b/scripts/git.orderfile
index 6f214f4aca7e..890857258d96 100644
--- a/scripts/git.orderfile
+++ b/scripts/git.orderfile
@@ -38,6 +38,7 @@ common/*.h
common/*/*.h
# Generator files.
+generator/*.mli
generator/*
# Source files.
Laszlo Ersek
2023-Apr-21 07:34 UTC
[Libguestfs] [libnbd PATCH v2 3/4] generator/utils: import the "spaces" func from libguestfs-common Std_utils
In "pr_wrap" we already print a sequence of spaces, and we'll
print more
in a subsequent patch. Copy the "spaces" function from
libguestfs-common
Std_utils, which lets us avoid open-coded loops.
"spaces" was added to libguestfs-common in commit e067192a90dc
("mllib:
Split ?Common_utils? into ?Std_utils? + ?Common_utils?.",
2017-07-10). The files declaring and defining "spaces" are licensed
under
the GPLv2+. Libnbd is licensed under the LGPLv2+, which is laxer. The
copyright holder for "spaces" is Richard W.M. Jones <rjones at
redhat.com>;
he has relicensed "spaces" for importing into libnbd
<https://listman.redhat.com/archives/libguestfs/2023-April/031344.html>.
Suggested-by: Richard W.M. Jones <rjones at redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
Notes:
v2:
- new patch [Rich]
generator/utils.mli | 1 +
generator/utils.ml | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/generator/utils.mli b/generator/utils.mli
index 62b86fddadbf..5bbb188fe934 100644
--- a/generator/utils.mli
+++ b/generator/utils.mli
@@ -45,6 +45,7 @@ val
val span : string -> string -> int
val cspan : string -> string -> int
val quote : string -> string
+val spaces : int -> string
val files_equal : string -> string -> bool
val generate_header : ?extra_sources:string list -> comment_style -> unit
diff --git a/generator/utils.ml b/generator/utils.ml
index 0121d92e1ab2..48bd6dd12ba8 100644
--- a/generator/utils.ml
+++ b/generator/utils.ml
@@ -149,6 +149,8 @@ let
| Buffer b -> Buffer.add_string b str
) fs
+let spaces n = String.make n ' '
+
(* Wrap the output at maxcol, breaking up lines when a 'c' character
* occurs. For example:
* foobar = a, b, c, d, e, f, g
@@ -179,8 +181,7 @@ let
let fields = nsplit (String.make 1 c) line in
let maybe_wrap field if !col > wrapping_col && !col +
String.length field >= maxcol then (
- pr "\n";
- for i = 0 to wrapping_col-1 do pr " " done;
+ pr "\n%s" (spaces wrapping_col);
match span field " \t" with
| 0 -> field
| i -> String.sub field i (String.length field - i)
Laszlo Ersek
2023-Apr-21 07:34 UTC
[Libguestfs] [libnbd PATCH v2 4/4] generator: indent C argument list 2 spaces relative to function designator
Change the optional "parens" parameter of "print_arg_list" from bool to a new union type. The new union type has three data constructors, NoParens (matching the previous "false" value), ParensSameLine (matching the previous "true" value), and a third constructor called "ParensNewLineWithIndent", which wraps an "int". [ParensNewLineWithIndent n] stands for the following formatting construct (with wrapping):> ret_type foobar ( > type1 param1, type2 param2, type3 param3, type4 param4, > type5 param5 > ); > <---n--->and without wrapping:> ret_type foobar ( > type1 param1, type2 param2, type3 param3, type4 param4, type5 param5 > ); > <---n--->Capture "n" (that is, the 0-based column where the function designator starts) in "print_call", and put the new formatting to use. In effect, still utilize "pr_wrap", for wrapping the argument list at comma (",") characters, but shift the left hand side of the wrapped block from the end of the function designator near the front of it. The use case is: if "foobar" is long, and at least one "typeN" is also long, previously we would get> ret_type extremely_long_verbose_foobar (extremely_long_verbose_type1 param1, > type2 param2, type3 param3, > type4 param4, type5 param5);That is, the first wrapping opportunity on a particular line would be too late already, making the source code too wide. This patch changes the generated files "include/libnbd.h" and "lib/unlocked.h". Taking the declaration responsible for the longest line in each of those header files, the declaration is reformatted as shown below: [include/libnbd.h]> -extern int nbd_aio_opt_list_meta_context_queries (struct nbd_handle *h, > - char **queries, > - nbd_context_callback context_callback, > - nbd_completion_callback completion_callback) > +extern int nbd_aio_opt_list_meta_context_queries ( > + struct nbd_handle *h, char **queries, > + nbd_context_callback context_callback, > + nbd_completion_callback completion_callback > + ) > LIBNBD_ATTRIBUTE_NONNULL (1, 2);[lib/unlocked.h]> -extern int nbd_unlocked_aio_opt_list_meta_context_queries (struct nbd_handle *h, > - char **queries, > - nbd_context_callback *context_callback, > - nbd_completion_callback *completion_callback) > +extern int nbd_unlocked_aio_opt_list_meta_context_queries ( > + struct nbd_handle *h, char **queries, > + nbd_context_callback *context_callback, > + nbd_completion_callback *completion_callback > + ) > LIBNBD_ATTRIBUTE_NONNULL (1, 2);Update the GoLang generator to stick with "no parens". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516 Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- Notes: v2: - rename "args_style" to "parens_style" [Rich] - rename ParensNewLine to ParensNewLineWithIndent [Rich] - use the "spaces" function for indentation [Rich] generator/C.mli | 3 ++- generator/C.ml | 25 +++++++++++++++----- generator/GoLang.ml | 4 ++-- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/generator/C.mli b/generator/C.mli index 215acc64f8a9..bc93cf896c96 100644 --- a/generator/C.mli +++ b/generator/C.mli @@ -18,6 +18,7 @@ *) type closure_style = Direct | AddressOf | Pointer +type parens_style = NoParens | ParensSameLine | ParensNewLineWithIndent of int val generate_lib_libnbd_syms : unit -> unit val generate_include_libnbd_h : unit -> unit @@ -28,7 +29,7 @@ val val generate_docs_api_flag_links_pod : unit -> unit val generate_docs_nbd_pod : string -> API.call -> unit -> unit val print_arg_list : ?wrap:bool -> ?maxcol:int -> - ?handle:bool -> ?types:bool -> ?parens:bool -> + ?handle:bool -> ?types:bool -> ?parens:parens_style -> ?closure_style:closure_style -> API.arg list -> API.optarg list -> unit val print_cbarg_list : ?wrap:bool -> ?maxcol:int -> diff --git a/generator/C.ml b/generator/C.ml index 044c046dcff1..61ca43d95db5 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -23,6 +23,7 @@ open Utils type closure_style = Direct | AddressOf | Pointer +type parens_style = NoParens | ParensSameLine | ParensNewLineWithIndent of int let generate_lib_libnbd_syms () generate_header HashStyle; @@ -134,15 +135,23 @@ let let optarg_attr_nonnull (OClosure _ | OFlags _) = [ false ] -let rec print_arg_list ?(wrap = false) ?maxcol ?handle ?types ?(parens = true) - ?closure_style args optargs - if parens then pr "("; +let rec print_arg_list ?(wrap = false) ?maxcol ?handle ?types + ?(parens = ParensSameLine) ?closure_style args optargs + (match parens with + | NoParens -> () + | ParensSameLine -> pr "(" + | ParensNewLineWithIndent indent -> pr "(\n%s" (spaces (indent + 2)) + ); if wrap then pr_wrap ?maxcol ',' (fun () -> print_arg_list' ?handle ?types ?closure_style args optargs) else print_arg_list' ?handle ?types ?closure_style args optargs; - if parens then pr ")" + (match parens with + | NoParens -> () + | ParensSameLine -> pr ")" + | ParensNewLineWithIndent indent -> pr "\n%s)" (spaces indent) + ) and print_arg_list' ?(handle = false) ?(types = true) ?(closure_style = Direct) args optargs @@ -237,8 +246,12 @@ and ) optargs let print_call ?wrap ?maxcol ?closure_style name args optargs ret - pr "%s nbd_%s " (type_of_ret ret) name; - print_arg_list ~handle:true ?wrap ?maxcol ?closure_style args optargs + pr "%s " (type_of_ret ret); + let designator_column = output_column () in + pr "nbd_%s " name; + print_arg_list ~handle:true ?wrap ?maxcol + ~parens:(ParensNewLineWithIndent designator_column) ?closure_style args + optargs let print_fndecl ?wrap ?closure_style name args optargs ret pr "extern "; diff --git a/generator/GoLang.ml b/generator/GoLang.ml index 3120b7f7c72f..4ab6b2626ae3 100644 --- a/generator/GoLang.ml +++ b/generator/GoLang.ml @@ -135,7 +135,7 @@ let pr "%s\n" ret_c_type; pr "_nbd_%s_wrapper (struct error *err,\n" name; pr " "; - C.print_arg_list ~wrap:true ~handle:true ~parens:false args optargs; + C.print_arg_list ~wrap:true ~handle:true ~parens:NoParens args optargs; pr ")\n"; pr "{\n"; pr "#ifdef LIBNBD_HAVE_NBD_%s\n" ucname; @@ -736,7 +736,7 @@ let let ret_c_type = C.type_of_ret ret in pr "%s _nbd_%s_wrapper (struct error *err,\n" ret_c_type name; pr " "; - C.print_arg_list ~wrap:true ~handle:true ~parens:false args optargs; + C.print_arg_list ~wrap:true ~handle:true ~parens:NoParens args optargs; pr ");\n"; ) handle_calls; pr "\n";