Printing LSN made easy

Hi All,
An LSN is printed using format "%X/%X" and passing (uint32) (lsn >>
32), (uint32) lsn as arguments to that format specifier. This pattern
is repeated at 180 places according to Cscope. I find it hard to
remember this pattern every time I have to print LSN. Usually I end up
copying from some other place. Finding such a place takes a few
minutes and might be error prone esp when the lsn to be printed is an
expression. If we ever have to change this pattern in future, we will
need to change all those 180 places.

The solution seems to be simple though. In the attached patch, I have
added two macros
#define LSN_FORMAT "%X/%X"
#define LSN_FORMAT_ARG(lsn) (uint32) ((lsn) >> 32), (uint32) (lsn)

which can be used instead.

E.g. the following change in the patch
@@ -261,8 +261,7 @@ page_header(PG_FUNCTION_ARGS)
{
char lsnchar[64];

- snprintf(lsnchar, sizeof(lsnchar), "%X/%X",
- (uint32) (lsn >> 32), (uint32) lsn);
+ snprintf(lsnchar, sizeof(lsnchar), LSN_FORMAT,
LSN_FORMAT_ARG(lsn));
values[0] = CStringGetTextDatum(lsnchar);

LSN_FORMAT_ARG expands to two comma separated arguments and is kinda
open at both ends but it's handy that way.

Off list Peter Eisentraut pointed out that we can not use these macros
in elog/ereport since it creates problems for translations. He
suggested adding functions which return strings and use %s when doing
so.

The patch has two functions pg_lsn_out_internal() which takes an LSN
as input and returns a palloc'ed string containing the string
representation of LSN. This may not be suitable in performance
critical paths and also may leak memory if not freed. So there's
another function pg_lsn_out_buffer() which takes LSN and a char array
as input, fills the char array with the string representation and
returns the pointer to the char array. This allows the function to be
used as an argument in printf/elog etc. Macro MAXPG_LSNLEN has been
extern'elized for this purpose.

Off list Craig Ringer suggested introducing a new format specifier
similar to %m for LSN but I did not get time to take a look at the
relevant code. AFAIU it's available only to elog/ereport, so may not
be useful generally. But teaching printf variants about the new format
would be the best solution. However, I didn't find any way to do that.

In that patch I have changed some random code to use one of the above
methods appropriately, demonstrating their usage. Given that we have
grown 180 places already, I think that something like would have been
welcome earlier. But given that replication code is being actively
worked upon, it may not be too late. As a precedence lfirst_node() and
its variants arrived when the corresponding pattern had been repeated
at so many places.

I think we should move pg_lsn_out_internal() and pg_lsn_out_buffer()
somewhere else. Ideally xlogdefs.c would have been the best place but
there's no such file. May be we add those functions in pg_lsn.c and
add their declarations i xlogdefs.h.

--
Best Wishes,
Ashutosh Bapat