Re: [PATCH v6 2/6] winedbg: Buffer output of GDB qXfer commands for proper slicing.

[Rémi Bernon]

This looks almost good too.

On 11/22/21 15:59, Jinoh Kang wrote:
> +static void packet_reply_xfer(struct gdb_context* gdbctx, const void *data, size_t datalen,
> +                              unsigned int off, unsigned int len)
> +{
> +    BOOL nonempty;
> +    size_t trunc_len;
> +
> +    packet_reply_open(gdbctx);
> +
> +    nonempty = (size_t)off < datalen;
> +    if (nonempty && (size_t)off + len < datalen)
> +        packet_reply_add(gdbctx, "m");
> +    else
> +        packet_reply_add(gdbctx, "l");
> +
> +    if (nonempty)
> +    {
> +        trunc_len = min((size_t)len, datalen - off);
> +        packet_reply_add_data(gdbctx, (const unsigned char *)data + off, trunc_len);
> +    }
> +
> +    packet_reply_close(gdbctx);
> +}
> +

With base an unsigned char* you could save a cast there too.

I also don't think you need to cast unsigned int to size_t, you could 
even make the off and len params, size_t directly.

(The local variables also are a bit unnecessary IMHO but probably a 
matter of taste)
> static enum packet_return packet_query(struct gdb_context* gdbctx)
> @@ -1960,9 +1962,12 @@ static enum packet_return packet_query(struct gdb_context* gdbctx)
>           {
>               if (!gdbctx->process) return packet_error;
>
> -            packet_reply_open_xfer(gdbctx);
>               packet_query_libraries(gdbctx);
> -            packet_reply_close_xfer(gdbctx, off, len);
> +            packet_reply_xfer(gdbctx,
> +                              gdbctx->qxfer_buffer.base,
> +                              gdbctx->qxfer_buffer.len,
> +                              off, len);
> +            reply_buffer_empty(&gdbctx->qxfer_buffer);
>               return packet_done;
>           }
>
> @@ -1970,9 +1975,12 @@ static enum packet_return packet_query(struct gdb_context* gdbctx)
>           {
>               if (!gdbctx->process) return packet_error;
>
> -            packet_reply_open_xfer(gdbctx);
>               packet_query_threads(gdbctx);
> -            packet_reply_close_xfer(gdbctx, off, len);
> +            packet_reply_xfer(gdbctx,
> +                              gdbctx->qxfer_buffer.base,
> +                              gdbctx->qxfer_buffer.len,
> +                              off, len);
> +            reply_buffer_empty(&gdbctx->qxfer_buffer);
>               return packet_done;
>           }
>
> @@ -1981,9 +1989,12 @@ static enum packet_return packet_query(struct gdb_context* gdbctx)
>               if (!gdbctx->process) return packet_error;
>               if (!(cpu = gdbctx->process->be_cpu)) return packet_error;
>
> -            packet_reply_open_xfer(gdbctx);
>               packet_query_target_xml(gdbctx, cpu);
> -            packet_reply_close_xfer(gdbctx, off, len);
> +            packet_reply_xfer(gdbctx,
> +                              gdbctx->qxfer_buffer.base,
> +                              gdbctx->qxfer_buffer.len,
> +                              off, len);
> +            reply_buffer_empty(&gdbctx->qxfer_buffer);
>               return packet_done;
>           }
>           break;

Maybe you could directly use gdbctx->qxfer_buffer in packet_reply_xfer, 
instead of passing base and len here?

-- 
Rémi Bernon <rbernon(a)codeweavers.com>