Re: [PATCH 11/16] docs: hacking: document string concatenations

[Jonathon Jongsma <jjongsma@xxxxxxxxxx>]

On Sat, 2019-10-19 at 00:36 +0200, Ján Tomko wrote:
> Recommend GString for generic strings and virBuffer for strings
> that need helpers for other uses, like XML or command line
> formatting.
> 
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
>  docs/hacking.html.in | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/hacking.html.in b/docs/hacking.html.in
> index d6a4f04ad0..384da96d60 100644
> --- a/docs/hacking.html.in
> +++ b/docs/hacking.html.in
> @@ -1289,7 +1289,11 @@ BAD:
>      <p>
>        If there is a need for complex string concatenations, avoid
> using
>        the usual sequence of malloc/strcpy/strcat/snprintf functions
> and
> -      make use of the virBuffer API described in virbuffer.h
> +      make use of either the
> +      <a href="
> https://developer.gnome.org/glib/stable/glib-Strings.html";>GString</a
> >
> +      type from GLib.


Not sure whether you've already pushed this or not, but this sentence
seems incomplete. "either" should be accompanied by an "or" within the
same sentence: 
 "You can use either A or B."

Here, you just have:
  "You can use either A."

> +      If formatting XML or QEMU command line is needed, use the
> virBuffer
> +      API described in virbuffer.h, since it has helper functions
> for those.
>      </p>
>  
>      <p>Typical usage is as follows:</p>
> @@ -1298,12 +1302,14 @@ BAD:
>    char *
>    somefunction(...)
>    {
> -     virBuffer buf = VIR_BUFFER_INITIALIZER;
> +     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>  
>       ...
>  
>       virBufferAddLit(&amp;buf, "&lt;domain&gt;\n");
>       virBufferAsprintf(&amp;buf,
> "  &lt;memory&gt;%d&lt;/memory&gt;\n", memory);
> +     if (some_error)
> +         return NULL; /* g_auto will free the memory used so far */
>       ...
>       virBufferAddLit(&amp;buf, "&lt;/domain&gt;\n");
>  
> @@ -1387,12 +1393,10 @@ BAD:
>      </p>
>  
>      <p>
> -      When printing to a string, consider using virBuffer for
> -      incremental allocations, virAsprintf for a one-shot
> allocation,
> -      and snprintf for fixed-width buffers.  Do not use sprintf,
> even
> -      if you can prove the buffer won't overflow, since gnulib does
> -      not provide the same portability guarantees for sprintf as it
> -      does for snprintf.
> +      When printing to a string, consider using GString or virBuffer
> for
> +      incremental allocations, g_strdup_printf for a one-shot
> allocation,
> +      and g_snprintf for fixed-width buffers.  Only use g_sprintf,
> +      if you can prove the buffer won't overflow.
>      </p>
>  
>      <h2><a id="errors">Error message format</a></h2>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list