Re: [PATCH] XLogReader v2

On Sunday, September 09, 2012 08:40:38 PM Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)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