Thread: StringInfo Macros

Hi,

trying to clean up the Postgres-R code further, I would like to make  
use of StringInfo and accompanying functions in libpq/pqformat.c  
instead of some home-brown duplicate of it. However, I'm missing  
helper macros like these (mainly for readability of the code):

#define BUFFER_START_PTR(buffer) ((buffer)->data))
#define BUFFER_END_PTR(buffer) ((buffer)->data + (buffer)->len)
#define BUFFER_CURSOR_PTR(buffer) ((buffer)->data + (buffer)->cursor)

#define BUFFER_BYTES_FREE(buffer) ((buffer)->maxlen - (buffer)->len)
#define BUFFER_BYTES_READABLE(buffer) ((buffer)->len - (buffer)->cursor)

Is there any compelling reason these don't exist (and shouldn't get  
added)? IF not, I'd be happy to create a patch to add and make use of  
such macros.

Regards

Markus Wanner

"Markus Wanner" <markus@bluegap.ch> writes:
> trying to clean up the Postgres-R code further, I would like to make  
> use of StringInfo and accompanying functions in libpq/pqformat.c  
> instead of some home-brown duplicate of it. However, I'm missing  
> helper macros like these (mainly for readability of the code):

> #define BUFFER_START_PTR(buffer) ((buffer)->data))
> #define BUFFER_END_PTR(buffer) ((buffer)->data + (buffer)->len)
> #define BUFFER_CURSOR_PTR(buffer) ((buffer)->data + (buffer)->cursor)

> #define BUFFER_BYTES_FREE(buffer) ((buffer)->maxlen - (buffer)->len)
> #define BUFFER_BYTES_READABLE(buffer) ((buffer)->len - (buffer)->cursor)

> Is there any compelling reason these don't exist (and shouldn't get  
> added)? IF not, I'd be happy to create a patch to add and make use of  
> such macros.

Don't call them BUFFER_FOO --- that term already has a pretty specific
meaning in most of the backend.  STRINGINFO_FOO is a bit long but
seems to fit the best with the existing naming in stringinfo.h.

A number of the specific things you are proposing above seem to be
violations of the abstraction stringinfo is meant to present.
It's a string, and the fact that there's any extra space beyond
the end of the string is an internal matter.  In particular I wonder
exactly what you think END_PTR or BYTES_FREE would be used for.

Also, the cursor field is meant to be manipulated directly by calling
code, so I'm not sure how much value there's going to be in abstracting
that.  CURSOR_PTR as shown above seems a bit dangerous --- it might
encourage someone to hold onto such a pointer across a StringInfo
operation that could move buffer->data.  Although maybe I'm overthinking
that, since someone could try that with or without a macro :-(
        regards, tom lane

Hello Tom,

Tom Lane wrote:
> Don't call them BUFFER_FOO --- that term already has a pretty specific
> meaning in most of the backend.  STRINGINFO_FOO is a bit long but
> seems to fit the best with the existing naming in stringinfo.h.

Understood and agreed. Note however, that lots of places, especially
also pgformat.c, call the StringInfo variable "buf".

> A number of the specific things you are proposing above seem to be
> violations of the abstraction stringinfo is meant to present.

Maybe in violation of stringinfo, which is more general, but much less
WRT all of the pq_sendXX() and the corresponding pq_getmsgXX() methods.
AFAIU the former don't use the cursor and write starting from END_PTR as
long as BYTES_FREE > 0, while the later uses the cursor field and reads
from CURSOR_PTR as long as BYTES_READABLE > 0.

> It's a string, and the fact that there's any extra space beyond
> the end of the string is an internal matter.

I rather think of those as buffers to read from and write to. In that
case it makes perfect sense to append new data at the end, IMO.

On could argue, that StringInfo itself isn't the best fit, as it doesn't
necessarily hold a string. So it might be hard to find a good prefix.
PQFORMAT_ might be an option, but it's even less descriptive...

Thank you for commenting.

Regards

Markus Wanner