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?