Re: [HACKERS] pg_upgrade bugs
On 09/03/2012 12:00 AM, Tom Lane wrote: Alvaro Herrera writes: Maybe, to reduce future backpatching pain, we could backpatch the change to exec_prog() API now that you have fixed the implementation? I'm inclined to think this is a good idea, but keep in mind we're less than four days from wrapping 9.2.0. There's not a lot of margin for error here. At the same time, getting pg_upgrade to pass regression on Windows would be a good thing, and ought to go a long way towards ameliorating worries about this. My first focus was on getting it working on HEAD, and I only got there fairly late last night. I'll look at getting testing working on 9.2 today. At this stage my plan is to make the absolute minimum changes necessary, but if Alvaro wants to do something a bit larger to align t code, I'll be happy to test it. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade bugs
Alvaro Herrera writes: > Maybe, to reduce future backpatching pain, we could backpatch the change > to exec_prog() API now that you have fixed the implementation? I'm inclined to think this is a good idea, but keep in mind we're less than four days from wrapping 9.2.0. There's not a lot of margin for error here. At the same time, getting pg_upgrade to pass regression on Windows would be a good thing, and ought to go a long way towards ameliorating worries about this. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade bugs
Excerpts from Andrew Dunstan's message of dom sep 02 22:11:42 -0300 2012: > > I have been wrestling for a couple of days trying to get pg_upgrade > testing working on Windows, with a view to having it tested on the > buildfarm. The test script has its own issues, which I'll deal with > separately, but there are two issues in pg_upgrade's exec.c that make me > suspect that if pg_upgrade has ever worked at all on Windows it is a > matter of sheer luck. The attached patch fixes these. The first issue is > a plain miscall to stlcpy(), where the length argument is wrong. The > second is where exec_prog tries to open a log file after the system call > returns. This will fail if the command was a 'pg_ctl start', as the > running postmaster will have the log file open, so I have simply > #ifdef'd it out for the Windows case, as the code does nothing except > add a couple of line feeds to the log, missing which won't affect > anything much. The first issue was clearly introduced by me in 088c065ce8e405fafbfa966937184ece9defcf20. The other one is probably also my fault. Bruce has tested this on Windows previously and fixed some previous bugs also introduced by me in the same area; so we know it works. Maybe, to reduce future backpatching pain, we could backpatch the change to exec_prog() API now that you have fixed the implementation? > Barring objection I will commit this before long. Thanks. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade bugs
Andrew Dunstan writes: > I have been wrestling for a couple of days trying to get pg_upgrade > testing working on Windows, with a view to having it tested on the > buildfarm. The test script has its own issues, which I'll deal with > separately, but there are two issues in pg_upgrade's exec.c that make me > suspect that if pg_upgrade has ever worked at all on Windows it is a > matter of sheer luck. The attached patch fixes these. The first issue is > a plain miscall to stlcpy(), where the length argument is wrong. The > second is where exec_prog tries to open a log file after the system call > returns. This will fail if the command was a 'pg_ctl start', as the > running postmaster will have the log file open, so I have simply > #ifdef'd it out for the Windows case, as the code does nothing except > add a couple of line feeds to the log, missing which won't affect > anything much. The strlcpy bug seems to be recently introduced --- I don't see it in 9.2. I think the other bit has not been there very long either, though it *is* in 9.2 branch so you'd better back-patch that part. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_upgrade bugs
I have been wrestling for a couple of days trying to get pg_upgrade
testing working on Windows, with a view to having it tested on the
buildfarm. The test script has its own issues, which I'll deal with
separately, but there are two issues in pg_upgrade's exec.c that make me
suspect that if pg_upgrade has ever worked at all on Windows it is a
matter of sheer luck. The attached patch fixes these. The first issue is
a plain miscall to stlcpy(), where the length argument is wrong. The
second is where exec_prog tries to open a log file after the system call
returns. This will fail if the command was a 'pg_ctl start', as the
running postmaster will have the log file open, so I have simply
#ifdef'd it out for the Windows case, as the code does nothing except
add a couple of line feeds to the log, missing which won't affect
anything much.
Barring objection I will commit this before long.
cheers
andrew
diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c
index c75d9db..c780b63 100644
--- a/contrib/pg_upgrade/exec.c
+++ b/contrib/pg_upgrade/exec.c
@@ -52,7 +52,7 @@ exec_prog(const char *log_file, const char *opt_log_file,
old_umask = umask(S_IRWXG | S_IRWXO);
- written = strlcpy(cmd, SYSTEMQUOTE, strlen(SYSTEMQUOTE));
+ written = strlcpy(cmd, SYSTEMQUOTE, sizeof(cmd));
va_start(ap, fmt);
written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap);
va_end(ap);
@@ -95,10 +95,12 @@ exec_prog(const char *log_file, const char *opt_log_file,
log_file);
}
+#ifndef WIN32
if ((log = fopen_priv(log_file, "a+")) == NULL)
pg_log(PG_FATAL, "cannot write to log file %s\n", log_file);
fprintf(log, "\n\n");
fclose(log);
+#endif
return result == 0;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
