Re: pg_archivecleanup debug message consistency

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Erik Rijkers" <er(at)xs4all(dot)nl>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_archivecleanup debug message consistency
Date: 2010-08-22 15:54:05
Message-ID: 16510.1282492445@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Erik Rijkers" <er(at)xs4all(dot)nl> writes:
> If only for consistency, this patch adds the path info to that message.

Seems reasonable, but speaking of consistency:

> +#ifdef WIN32
> + snprintf(WALFilePath, MAXPGPATH, "%s\\%s", archiveLocation, exclusiveCleanupFileName);
> +#else
> + snprintf(WALFilePath, MAXPGPATH, "%s/%s", archiveLocation, exclusiveCleanupFileName);
> +#endif

I see that you copied-and-pasted this pattern from somewhere else in
pg_archivecleanup.c, but I'd like to argue that it's out of place there
too. We don't go out of our way to show Windows paths with backslashes
anywhere in the core code, so why is pg_archivecleanup doing it? I
think we should just drop the ifdef and do %s/%s always.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2010-08-22 15:57:21 Re: Replacing the pg_get_expr security hack with a datatype solution
Previous Message Tom Lane 2010-08-22 15:29:33 Re: More vacuum stats