On Tue, Jul 14, 2020 at 7:26 AM Amit Langote <amitlangot...@gmail.com> wrote:
>
> Good idea, thanks.
>
> In CopyLoadRawBuf(), we could also change the condition if
> (cstate->raw_buf_index < cstate->raw_buf_len) to if (BUF_BYTES > 0),
> which looks clearer.
>
> Also, if we are going to use the macro more generally, let's make it
> look less localized. For example, rename it to RAW_BUF_BYTES similar
> to RAW_BUF_SIZE and place their definitions close by. It also seems
> like a good idea to make 'cstate' a parameter for clarity.
>
> Attached v6.
>
Thanks for making the changes.
- if (cstate->raw_buf_index < cstate->raw_buf_len)
+ if (RAW_BUF_BYTES(cstate) > 0)
{
/* Copy down the unprocessed data */
- nbytes = cstate->raw_buf_len - cstate->raw_buf_index;
+ nbytes = RAW_BUF_BYTES(cstate);
memmove(cstate->raw_buf, cstate->raw_buf +
cstate->raw_buf_index,
nbytes);
}
One small improvement could be to change it like below to reduce few
more instructions:
static bool
CopyLoadRawBuf(CopyState cstate)
{
int nbytes = RAW_BUF_BYTES(cstate);
int inbytes;
/* Copy down the unprocessed data */
if (nbytes > 0)
memmove(cstate->raw_buf, cstate->raw_buf + cstate->raw_buf_index,
nbytes);
inbytes = CopyGetData(cstate, cstate->raw_buf + nbytes,
1, RAW_BUF_SIZE - nbytes);
nbytes += inbytes;
cstate->raw_buf[nbytes] = '\0';
cstate->raw_buf_index = 0;
cstate->raw_buf_len = nbytes;
return (inbytes > 0);
}
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
