[PATCHES] Win32 fix for pg_dumpall
Here is fix for Win32 pg_dumpall that Claudio helped with.
Attached and applied.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/port/exec.c
===
RCS file: /cvsroot/pgsql-server/src/port/exec.c,v
retrieving revision 1.18
diff -c -c -r1.18 exec.c
*** src/port/exec.c 8 Aug 2004 02:22:55 - 1.18
--- src/port/exec.c 8 Aug 2004 03:16:52 -
***
*** 374,381
CloseHandle(childstdoutrddup);
return NULL;
}
!
!
/* We try just once */
if (ReadFile(childstdoutrddup, line, maxsize, &bytesread, NULL) &&
bytesread > 0)
--- 374,380
CloseHandle(childstdoutrddup);
return NULL;
}
!
/* We try just once */
if (ReadFile(childstdoutrddup, line, maxsize, &bytesread, NULL) &&
bytesread > 0)
***
*** 384,389
--- 383,402
retval = line;
/*
+* Sometime the child returns "\r\n", which doesn't match
+* our version string. The backend uses
+* setvbuf(stdout, NULL, _IONBF, 0), but pg_dump doesn't
+* so we have to fix it here.
+*/
+ if (strlen(line) >= 2 &&
+ line[strlen(line)-2] == '\r' &&
+ line[strlen(line)-1] == '\n')
+ {
+ line[strlen(line)-2] == '\n';
+ line[strlen(line)-1] == '\0';
+ }
+
+ /*
* We emulate fgets() behaviour. So if there is no newline
* at the end, we add one...
*/
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Win32 fix for pg_dumpall
Bruce Momjian wrote:
/*
+ * Sometime the child returns "\r\n", which doesn't match
+ * our version string. The backend uses
+ * setvbuf(stdout, NULL, _IONBF, 0), but pg_dump doesn't
+ * so we have to fix it here.
+ */
+ if (strlen(line) >= 2 &&
+ line[strlen(line)-2] == '\r' &&
+ line[strlen(line)-1] == '\n')
+ {
+ line[strlen(line)-2] == '\n';
+ line[strlen(line)-1] == '\0';
+ }
+
+ /*
I do not see how the comment relates at all to the code following it -
buffer mode and line end mode are two different things. Also, the
repeated calls to strlen(line) are horribly inefficient - it should be
called once and stashed in an int (I once made an order of magnitude
speedup in a program by correcting a piece of someone else's code that
looked like this: for (i = 0; i <= strlen(s); i++) where s was an
invariant very long string)
cheers
andrew
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Win32 fix for pg_dumpall
Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
>
> >
> > /*
> >+ * Sometime the child returns "\r\n", which doesn't match
> >+ * our version string. The backend uses
> >+ * setvbuf(stdout, NULL, _IONBF, 0), but pg_dump doesn't
> >+ * so we have to fix it here.
> >+ */
> >+if (strlen(line) >= 2 &&
> >+line[strlen(line)-2] == '\r' &&
> >+line[strlen(line)-1] == '\n')
> >+{
> >+line[strlen(line)-2] == '\n';
> >+line[strlen(line)-1] == '\0';
> >+}
> >+
> >+/*
> >
> >
> >
>
> I do not see how the comment relates at all to the code following it -
> buffer mode and line end mode are two different things. Also, the
Yea, you would _think_ they are unrelated on Win32, but they aren't.
:-)
Turns out when you do that call in the backend, all EOLs become \n and
not \r\n. This is what Claudio found. Let me document the strangeness
of this more clearly.
> repeated calls to strlen(line) are horribly inefficient - it should be
> called once and stashed in an int (I once made an order of magnitude
> speedup in a program by correcting a piece of someone else's code that
> looked like this: for (i = 0; i <= strlen(s); i++) where s was an
> invariant very long string)
OK, I will clean that up too, but after beta1.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] Win32 fix for pg_dumpall
OK, I have cleaned up this code and clarified the comment. Attached and
applied.
---
Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
>
> >
> > /*
> >+ * Sometime the child returns "\r\n", which doesn't match
> >+ * our version string. The backend uses
> >+ * setvbuf(stdout, NULL, _IONBF, 0), but pg_dump doesn't
> >+ * so we have to fix it here.
> >+ */
> >+if (strlen(line) >= 2 &&
> >+line[strlen(line)-2] == '\r' &&
> >+line[strlen(line)-1] == '\n')
> >+{
> >+line[strlen(line)-2] == '\n';
> >+line[strlen(line)-1] == '\0';
> >+}
> >+
> >+/*
> >
> >
> >
>
> I do not see how the comment relates at all to the code following it -
> buffer mode and line end mode are two different things. Also, the
> repeated calls to strlen(line) are horribly inefficient - it should be
> called once and stashed in an int (I once made an order of magnitude
> speedup in a program by correcting a piece of someone else's code that
> looked like this: for (i = 0; i <= strlen(s); i++) where s was an
> invariant very long string)
>
> cheers
>
> andrew
>
> ---(end of broadcast)---
> TIP 4: Don't 'kill -9' the postmaster
>
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/port/exec.c
===
RCS file: /cvsroot/pgsql-server/src/port/exec.c,v
retrieving revision 1.21
diff -c -c -r1.21 exec.c
*** src/port/exec.c 9 Aug 2004 20:20:46 - 1.21
--- src/port/exec.c 16 Aug 2004 01:24:38 -
***
*** 381,406
{
/* So we read some data */
retval = line;
/*
!* Sometime the child returns "\r\n", which doesn't match
!* our version string. The backend uses
!* setvbuf(stdout, NULL, _IONBF, 0), but pg_dump doesn't
!* so we have to fix it here.
*/
! if (strlen(line) >= 2 &&
! line[strlen(line)-2] == '\r' &&
! line[strlen(line)-1] == '\n')
{
! line[strlen(line)-2] = '\n';
! line[strlen(line)-1] = '\0';
}
/*
* We emulate fgets() behaviour. So if there is no newline
* at the end, we add one...
*/
! if (line[strlen(line)-1] != '\n')
strcat(line,"\n");
}
--- 381,408
{
/* So we read some data */
retval = line;
+ int len = strlen(line);
/*
!* If EOL is \r\n, convert to just \n.
!* Because stdout is a text-mode stream, the \n output by
!* the child process is received as \r\n, so we convert it
!* to \n. The server main.c sets
!* setvbuf(stdout, NULL, _IONBF, 0) which has the effect
!* of disabling \n to \r\n expansion for stdout.
*/
! if (len >= 2 && line[len-2] == '\r' && line[len-1] == '\n')
{
! line[len-2] = '\n';
! line[len-1] = '\0';
! len--;
}
/*
* We emulate fgets() behaviour. So if there is no newline
* at the end, we add one...
*/
! if (line[len-1] != '\n')
strcat(line,"\n");
}
---(end of broadcast)---
TIP 8: explain analyze is your friend
Re: [PATCHES] Win32 fix for pg_dumpall
Bruce Momjian <[EMAIL PROTECTED]> writes: > /* >* We emulate fgets() behaviour. So if there is no newline >* at the end, we add one... >*/ > ! if (line[len-1] != '\n') > strcat(line,"\n"); > } This is untrustworthy if len is zero. Perhaps if (len == 0 || line[len-1] != '\n') strcat(line,"\n"); regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Win32 fix for pg_dumpall
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > /* > > * We emulate fgets() behaviour. So if there is no newline > > * at the end, we add one... > > */ > > ! if (line[len-1] != '\n') > > strcat(line,"\n"); > > } > > This is untrustworthy if len is zero. Perhaps > > if (len == 0 || line[len-1] != '\n') > strcat(line,"\n"); Agreed, fixed. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
