Re: [HACKERS] Patch to implement pg_current_logfile() function

On Wed, 15 Feb 2017 15:23:00 -0500
Robert Haas <robertmhaas@gmail.com> wrote:

> +            ereport(WARNING,
> +                    (errcode(ERRCODE_INTERNAL_ERROR),
> +                     errmsg("corrupted data found in \"%s\"",
> +                            LOG_METAINFO_DATAFILE)));
> 
> elog seems fine here.  There's no need for this to be translatable.
> Also, I'd change the level to ERROR.
> 
> +                     errhint("The supported log formats are
> \"stderr\""
> +                                    " and \"csvlog\".")));
> 
> I think our preferred style is not to break strings across lines like
> this.
> 
> +        log_filepath[strcspn(log_filepath, "\n")] = '\0';
> 
> We have no existing dependency on strcspn().  It might be better not
> to add one just for this feature; I suggest using strchr() instead,
> which is widely used in our existing code.


Attached is a v29 patch which fixes the above problems.

The Syslogger hunk remains to be fixed.  I have no plans
to do so at this time.

Note that since I have to write an "if" anyway, I'm going ahead
and raising an error condition when there's no newline in the
current_logfiles file.   The strcspn() ignored the missing newline.
The new code could do so as well by negating the if condition
should that be preferable.


On a different topic, I pulled from master and now
I see that git finds the following untracked:

    src/bin/pg_basebackup/pg_receivexlog
    src/bin/pg_resetxlog/
    src/bin/pg_xlogdump/

I'd appreciate knowing if I'm doing something dumb
on my end to make this happen.  Thanks.

Karl <kop@meme.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers