Re: [PATCH] XLogReader v2 - Mailing list pgsql-hackers

From Andres Freund
Subject Re: [PATCH] XLogReader v2
Date
Msg-id 201209092214.31279.andres@2ndquadrant.com
Whole thread Raw
In response to Re: [PATCH] XLogReader v2  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Sunday, September 09, 2012 08:40:38 PM Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > On Tuesday, September 04, 2012 09:33:54 PM Alvaro Herrera wrote:
> >> * There are way too many #ifdef VERBOSE_DEBUG stuff for my taste.  It
> >> might look better if you had macros such as elog_debug() that are
> >> defined to empty if VERBOSE_DEBUG is not defined.  (The problem with
> >> such an approach is that you have to get into the business of creating
> >> one macro for each different param count, so elog_debug1(),
> >> elog_debug2() and so on.  It also means you have to count the number of
> >> args in each call to ensure you're calling the right one.)
> > 
> > Hm. I am generally not very happy with the logging as is. I don't want to
> > rely on elog() at all because that means the code suddently depends on
> > just about the whole backend which sucks (see my god ulgy makefile hack
> > for that...).
> 
> elog/ereport are already basically macros.  Can't they be redefined for
> use in a standalone program, with just minimal backing code?
True, its not too hard. I had a *very minimal* version that just forwarded to 
vfprintf before ditching that because I needed to link to *_desc anyway.

Its a bit ugly though if you want to use the same object file for backend and 
standalone code. It means everybody using XLogReader's logging output is tied 
to elog internals.

> > If we were to use that approach is there a platform that stops us from
> > using vararg macros? I *think* it is C99...
> 
> C90 is still the project standard, and this is a pretty lame reason to
> want to change it.
Well, for the most part its a debugging utility, nothing enabled during normal 
builds... But I don't think its an important issue, if it comes to that we can 
do it just the same as elog.h does it. I.e. using a parameterless macro.

> >> * In the code beautification front, there are a number of cuddled braces
> >> and improperly indented function declarations.
> > 
> > I never seem to get those right. I really tried to make a pass over the
> > whole file correcting them...
> 
> Install pgindent?
I have, but it so often generates too much noise in unrelated parts that I 
stopped bothering. Which is a bad excuse in this case because its a new 
file...

Andres

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



pgsql-hackers by date:

Previous
From: Stefan Kaltenbrunner
Date:
Subject: Re: Draft release notes complete
Next
From: Peter Eisentraut
Date:
Subject: Re: build farm machine using mixed results