[Libguestfs] [libnbd PATCH 2/2] debug: Trace even may_set_error=false functions

[Eric Blake]

On Sun, Sep 04, 2022 at 05:44:33PM +0100, Richard W.M. Jones wrote:
> 
> So my feeling about this patch series:
> 
> I don't understand why the first patch is necessary, _and_ I think it
> might be "dangerous" (for small values of dangerous).
> 
> What happens if in future we add a new RStaticString API which returns
> a string that does need to be escaped?  It should at least be
> documented that RStaticString must only return simple, printable
> strings.  But ideally the first patch wouldn't exist and we could deal
> with any RStaticString API.

At present, the only escaping we do is
nbd_internal_printable_string(), which converts NULL to "NULL" (not
relevant here; either may_set_error=true and we already filter out
NULL as an error, or may_set_error=false and the result is guaranteed
non-NULL), and which truncates strings longer than 512 bytes to avoid
overlong logs.  But RStaticString returns are going to be a
compile-time constant string, and we aren't going to write a string
that long that would need our current form of escaping.

> 
> Second patch is completely fine.

I had already pushed both patches.  But if we revert the first one,
using just the second, I was having difficulties making the generated
api.c line up nicely.  It was looking something like:


  if_debug (h) {
      char *ret_printable =
          nbd_internal_printable_string (ret);
    debug_direct (h, "nbd_get_tls", "leave: ret=" "%s", ret_printable ? ret_printable : "");
      free (ret_printable);
  }


where the detour through nbd_internal_printable_string() with its
extra spacing wasn't making much sense compared to just printing it
directly by removing the special handling for RStaticString in
general.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org