Patch applied. ---------------------------------------------------------------------------
Bruce Momjian wrote: > I found a bug in the pg_restore code. It shows up only using the tar > format, and only on Windows XP (not Win2000 or BSD/OS). However, the > bug exists on all platforms that don't return zero'ed memory from > malloc, which is basically everyone. We have gotten away with this > because malloc memory is usually zeroed by accident in pg_dump (I think > because it is an early malloc call, not recycled from a free.) > > The bug seems to only affect the output displayed by pg_restore --- the > data seems to restore fine. To test, try: > > pg_dump -Ft test >/tmp/test.db > pg_dump /tmp/test.db > > For a simple case, you should see displayed by pg_restore: > > COPY supplier (sno, sname, city) FROM stdin; > 1 Smith London > 2 Jones Paris > 3 Adams Vienna > 4 Blake Roma > \. > > But on XP with the bug I see: > > COPY supplier (sno, sname, city) FROM stdin; > \. > copy supplier (sno, sname, city) from '$$PATH$$/6.dat' ; > > The code in pg_backup_tar.c::InitArchiveFmt_Tar does: > > ctx = (lclContext *) malloc(sizeof(lclContext)); > AH->formatData = (void *) ctx; > ctx->filePos = 0; > > What it doesn't do is to set ctx->isSpecialScript to zero: > > ctx->isSpecialScript = 0; > > pg_backup_tar::_PrintTocData() checks ctx->isSpecialScript for non-zero, > and then uses the wrong code path on XP. The code is supposed to use > the ctx->isSpecialScript code path only after the archive is closed. > This is set to true in _CloseArchive(). ctx->isSpecialScript is used > only for tar, so that's why only tar has the bug. > > A simple patch would be to just add the ctx->isSpecialScript = 0, but a > more reliable patch would be to do that, plus change a few malloc calls > to calloc for complex structures where the initialization isn't clear in > the code. > > I would like to apply the attached patch to 7.3.X and 7.4. > > Comments? > > -- > 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/bin/pg_dump/pg_backup_custom.c > =================================================================== > RCS file: /cvsroot/pgsql-server/src/bin/pg_dump/pg_backup_custom.c,v > retrieving revision 1.25 > diff -c -c -r1.25 pg_backup_custom.c > *** src/bin/pg_dump/pg_backup_custom.c 4 Aug 2003 00:43:27 -0000 1.25 > --- src/bin/pg_dump/pg_backup_custom.c 6 Oct 2003 18:00:40 -0000 > *************** > *** 136,142 **** > /* > * Set up some special context used in compressing data. > */ > ! ctx = (lclContext *) malloc(sizeof(lclContext)); > if (ctx == NULL) > die_horribly(AH, modulename, "out of memory\n"); > AH->formatData = (void *) ctx; > --- 136,142 ---- > /* > * Set up some special context used in compressing data. > */ > ! ctx = (lclContext *) calloc(1, sizeof(lclContext)); > if (ctx == NULL) > die_horribly(AH, modulename, "out of memory\n"); > AH->formatData = (void *) ctx; > *************** > *** 253,259 **** > > if (ctx == NULL) > { > ! ctx = (lclTocEntry *) malloc(sizeof(lclTocEntry)); > te->formatData = (void *) ctx; > } > > --- 253,259 ---- > > if (ctx == NULL) > { > ! ctx = (lclTocEntry *) calloc(1, sizeof(lclTocEntry)); > te->formatData = (void *) ctx; > } > > Index: src/bin/pg_dump/pg_backup_files.c > =================================================================== > RCS file: /cvsroot/pgsql-server/src/bin/pg_dump/pg_backup_files.c,v > retrieving revision 1.21 > diff -c -c -r1.21 pg_backup_files.c > *** src/bin/pg_dump/pg_backup_files.c 25 Oct 2002 01:33:17 -0000 1.21 > --- src/bin/pg_dump/pg_backup_files.c 6 Oct 2003 18:00:40 -0000 > *************** > *** 101,107 **** > /* > * Set up some special context used in compressing data. > */ > ! ctx = (lclContext *) malloc(sizeof(lclContext)); > AH->formatData = (void *) ctx; > ctx->filePos = 0; > > --- 101,107 ---- > /* > * Set up some special context used in compressing data. > */ > ! ctx = (lclContext *) calloc(1, sizeof(lclContext)); > AH->formatData = (void *) ctx; > ctx->filePos = 0; > > *************** > *** 167,173 **** > lclTocEntry *ctx; > char fn[K_STD_BUF_SIZE]; > > ! ctx = (lclTocEntry *) malloc(sizeof(lclTocEntry)); > if (te->dataDumper) > { > #ifdef HAVE_LIBZ > --- 167,173 ---- > lclTocEntry *ctx; > char fn[K_STD_BUF_SIZE]; > > ! ctx = (lclTocEntry *) calloc(1, sizeof(lclTocEntry)); > if (te->dataDumper) > { > #ifdef HAVE_LIBZ > *************** > *** 206,212 **** > > if (ctx == NULL) > { > ! ctx = (lclTocEntry *) malloc(sizeof(lclTocEntry)); > te->formatData = (void *) ctx; > } > > --- 206,212 ---- > > if (ctx == NULL) > { > ! ctx = (lclTocEntry *) calloc(1, sizeof(lclTocEntry)); > te->formatData = (void *) ctx; > } > > Index: src/bin/pg_dump/pg_backup_tar.c > =================================================================== > RCS file: /cvsroot/pgsql-server/src/bin/pg_dump/pg_backup_tar.c,v > retrieving revision 1.37 > diff -c -c -r1.37 pg_backup_tar.c > *** src/bin/pg_dump/pg_backup_tar.c 4 Aug 2003 00:43:27 -0000 1.37 > --- src/bin/pg_dump/pg_backup_tar.c 6 Oct 2003 18:00:41 -0000 > *************** > *** 158,167 **** > /* > * Set up some special context used in compressing data. > */ > ! ctx = (lclContext *) malloc(sizeof(lclContext)); > AH->formatData = (void *) ctx; > ctx->filePos = 0; > ! > /* Initialize LO buffering */ > AH->lo_buf_size = LOBBUFSIZE; > AH->lo_buf = (void *) malloc(LOBBUFSIZE); > --- 158,168 ---- > /* > * Set up some special context used in compressing data. > */ > ! ctx = (lclContext *) calloc(1, sizeof(lclContext)); > AH->formatData = (void *) ctx; > ctx->filePos = 0; > ! ctx->isSpecialScript = 0; > ! > /* Initialize LO buffering */ > AH->lo_buf_size = LOBBUFSIZE; > AH->lo_buf = (void *) malloc(LOBBUFSIZE); > *************** > *** 253,259 **** > lclTocEntry *ctx; > char fn[K_STD_BUF_SIZE]; > > ! ctx = (lclTocEntry *) malloc(sizeof(lclTocEntry)); > if (te->dataDumper != NULL) > { > #ifdef HAVE_LIBZ > --- 254,260 ---- > lclTocEntry *ctx; > char fn[K_STD_BUF_SIZE]; > > ! ctx = (lclTocEntry *) calloc(1, sizeof(lclTocEntry)); > if (te->dataDumper != NULL) > { > #ifdef HAVE_LIBZ > *************** > *** 292,298 **** > > if (ctx == NULL) > { > ! ctx = (lclTocEntry *) malloc(sizeof(lclTocEntry)); > te->formatData = (void *) ctx; > } > > --- 293,299 ---- > > if (ctx == NULL) > { > ! ctx = (lclTocEntry *) calloc(1, sizeof(lclTocEntry)); > te->formatData = (void *) ctx; > } > > Index: src/bin/pg_dump/pg_dump.c > =================================================================== > RCS file: /cvsroot/pgsql-server/src/bin/pg_dump/pg_dump.c,v > retrieving revision 1.352 > diff -c -c -r1.352 pg_dump.c > *** src/bin/pg_dump/pg_dump.c 27 Sep 2003 22:10:01 -0000 1.352 > --- src/bin/pg_dump/pg_dump.c 6 Oct 2003 18:00:47 -0000 > *************** > *** 1078,1084 **** > write_msg(NULL, "preparing to dump the contents of > table %s\n", > classname); > > ! dumpCtx = (DumpContext *) malloc(sizeof(DumpContext)); > dumpCtx->tblinfo = (TableInfo *) tblinfo; > dumpCtx->tblidx = i; > dumpCtx->oids = oids; > --- 1078,1084 ---- > write_msg(NULL, "preparing to dump the contents of > table %s\n", > classname); > > ! dumpCtx = (DumpContext *) calloc(1, sizeof(DumpContext)); > dumpCtx->tblinfo = (TableInfo *) tblinfo; > dumpCtx->tblidx = i; > dumpCtx->oids = oids; > *************** > *** 1938,1946 **** > > *numFuncs = ntups; > > ! finfo = (FuncInfo *) malloc(ntups * sizeof(FuncInfo)); > ! > ! memset((char *) finfo, 0, ntups * sizeof(FuncInfo)); > > i_oid = PQfnumber(res, "oid"); > i_proname = PQfnumber(res, "proname"); > --- 1938,1944 ---- > > *numFuncs = ntups; > > ! finfo = (FuncInfo *) calloc(ntups, sizeof(FuncInfo)); > > i_oid = PQfnumber(res, "oid"); > i_proname = PQfnumber(res, "proname"); > *************** > *** 2144,2151 **** > * dumping only one, because we don't yet know which tables might be > * inheritance ancestors of the target table. > */ > ! tblinfo = (TableInfo *) malloc(ntups * sizeof(TableInfo)); > ! memset(tblinfo, 0, ntups * sizeof(TableInfo)); > > i_reloid = PQfnumber(res, "oid"); > i_relname = PQfnumber(res, "relname"); > --- 2142,2148 ---- > * dumping only one, because we don't yet know which tables might be > * inheritance ancestors of the target table. > */ > ! tblinfo = (TableInfo *) calloc(ntups, sizeof(TableInfo)); > > i_reloid = PQfnumber(res, "oid"); > i_relname = PQfnumber(res, "relname"); > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org -- 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 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
