On Mon, Sep 05, 2022 at 02:41:57PM -0500, Eric Blake wrote:
> 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.
It actually hexdumps the buffer (which is a bit weird actually). So I
think you're right here that printing it unconditionally is correct.
Sorry for the noise!
> >
> > 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.
Let's not worry, thanks.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs
