On Mon, Jul 13, 2020 at 11:58 PM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
> > I had one small comment:
> > +{
> > + int copied_bytes = 0;
> > +
> > +#define BUF_BYTES (cstate->raw_buf_len - cstate->raw_buf_index)
> > +#define DRAIN_COPY_RAW_BUF(cstate, dest, nbytes)\
> > + do {\
> > + memcpy((dest), (cstate)->raw_buf +
> > (cstate)->raw_buf_index, (nbytes));\
> > + (cstate)->raw_buf_index += (nbytes);\
> > + } while(0)
> >
> > BUF_BYTES could be used in CopyLoadRawBuf function also.
> >
>
> Thanks Vignesh for the find out. I changed and attached the v5 patch.
> The regression tests(make check and make check-world) ran
> successfully.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. -- Amit Langote EnterpriseDB: http://www.enterprisedb.com
CopyFrom-binary-buffering_v6.patch
Description: Binary data
