[PATCH v3 6/9] winedbg: Buffer and escape output of GDB qXfer commands.

[Rémi Bernon]

On 11/16/21 17:51, Jinoh Kang wrote:
> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
> ---
>   programs/winedbg/gdbproxy.c | 284 +++++++++++++++++++++++++++---------
>   1 file changed, 211 insertions(+), 73 deletions(-)
> 
> diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c
> index bdb73659ade..caf64983c49 100644
> --- a/programs/winedbg/gdbproxy.c
> +++ b/programs/winedbg/gdbproxy.c
> @@ -54,6 +54,13 @@ struct gdb_xpoint
>       unsigned int value;
>   };
>   
> +struct vl_buffer
> +{
> +    void *base;
> +    size_t len;
> +    size_t alloc;
> +};
> +
>   struct gdb_context
>   {
>       /* gdb information */
> @@ -82,6 +89,7 @@ struct gdb_context
>       /* Unix environment */
>       ULONG_PTR                   wine_segs[3];   /* load addresses of the ELF wine exec segments (text, bss and data) */
>       BOOL                        no_ack_mode;
> +    struct vl_buffer            qxfer_buffer;
>   };
>   
>   /* assume standard signal and errno values */
> @@ -224,6 +232,102 @@ static void hex_to(char* dst, const void* src, size_t len)
>       }
>   }
>   
> +static void* buffer_realloc(void* buf, size_t size);
> +
> +static void vl_resize(struct vl_buffer* vlbuf, size_t alloc)
> +{
> +    vlbuf->alloc = alloc;
> +    vlbuf->base = buffer_realloc(vlbuf->base, vlbuf->alloc);
> +}
> +
> +static void vl_empty(struct vl_buffer *vlbuf)
> +{
> +    vlbuf->len = 0;
> +    vl_resize(vlbuf, 0);
> +}
> +
> +static void vl_grow(struct vl_buffer* vlbuf, size_t size)
> +{
> +    if (vlbuf->alloc < vlbuf->len + size)
> +        vl_resize(vlbuf, ((vlbuf->len + size) / 32 + 1) * 32);
> +}
> +
> +static void vl_append(struct vl_buffer* vlbuf, const void *data, size_t size)
> +{
> +    vl_grow(vlbuf, size);
> +    memcpy((void *)((unsigned char *)vlbuf->base + vlbuf->len), data, size);
> +    vlbuf->len += size;
> +}
> +
> +static inline void vl_append_str(struct vl_buffer* vlbuf, const char* str)
> +{
> +    vl_append(vlbuf, (const void *)str, strlen(str));
> +}
> +
> +static inline void vl_append_uinthex(struct vl_buffer* vlbuf, ULONG_PTR val, int len)
> +{
> +    char buf[sizeof(ULONG_PTR) * 2], *ptr;
> +
> +    assert(len <= sizeof(ULONG_PTR));
> +
> +    for (ptr = buf + len * 2; ptr != buf; val >>= 4)
> +        *--ptr = hex_to0(val & 0x0F);
> +
> +    vl_append(vlbuf, ptr, len * 2);
> +}
> +
> +static const unsigned char xml_special_chars_lookup_table[16] = {
> +    /* The characters should be sorted by its value modulo table length. */
> +
> +    0x00,       /* NUL */
> +    0,
> +    0x22,       /* ": 0010|0010 */
> +    0, 0, 0,
> +    0x26,       /* &: 0010|0110 */
> +    0x27,       /* ': 0010|0111 */
> +    0, 0, 0, 0,
> +    0x3C,       /* <: 0011|1100 */
> +    0,
> +    0x3E,       /* >: 0011|1110 */
> +    0
> +};
> +
> +static inline BOOL is_nul_or_xml_special_char(unsigned char val)
> +{
> +    /* Note: strcspn() uses lookup tables as well, but as of Wine 6.19
> +     * msvcrt!strcspn allocates 1024 bytes (sizeof(BOOL)*256) of table
> +     * on the stack and populates it on the fly.  It would be slower and less
> +     * cache-friendly than a preallocated, tiny static lookup table.
> +     */
> +
> +    const size_t length = ARRAY_SIZE(xml_special_chars_lookup_table);
> +    return xml_special_chars_lookup_table[val % length] == val;
> +}
> +
> +static void vl_append_xmlstr(struct vl_buffer* vlbuf, const char *str)
> +{
> +    const char *ptr = str, *sptr;
> +
> +    for (;;)
> +    {
> +        for (sptr = ptr; !is_nul_or_xml_special_char((unsigned char)*sptr); sptr++)
> +            ;
> +
> +        vl_append(vlbuf, ptr, sptr - ptr);
> +        ptr = sptr;
> +
> +        switch (*ptr++)
> +        {
> +        case '"': vl_append_str(vlbuf, """); break;
> +        case '&': vl_append_str(vlbuf, "&"); break;
> +        case '\'': vl_append_str(vlbuf, "'"); break;
> +        case '<': vl_append_str(vlbuf, "<"); break;
> +        case '>': vl_append_str(vlbuf, ">"); break;
> +        default: return;
> +        }
> +    }
> +}

There too, I find the for loop less readable than a more usual

  while (...) ptr++;

[...]

> +static void packet_reply_xfer(
> +    struct gdb_context* gdbctx,
> +    const void *data,
> +    size_t datalen,
> +    unsigned int off,
> +    unsigned int len,
> +    BOOL *more_p
> +)

FWIW I've seen other places where it's written like this, but Wine code 
is more usually breaking long lines on a (flexible) column limit, not on 
commas.

It's also usually aligned with the previous parenthese, (or using two 
indentation levels, but looking around in windbg, I think it's more 
appropriate to use tha aligned style here).
-- 
Rémi Bernon <rbernon at codeweavers.com>