Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit8980c72

Browse files
pg_dump: Fix compression API errorhandling
Compression in pg_dump is abstracted using an API with multipleimplementations which can be selected at runtime by the user.The API and its implementations have evolved over time, notablecommits includebf9aa49,e996073,84adc8e, and0da243f.The errorhandling defined by the API was however problematic andthe implementations had a few bugs and/or were not following theAPI specification. This commit modifies the API to ensure thatcallers can perform errorhandling efficiently and fixes all theimplementations such that they all implement the API in the sameway. A full list of the changes can be seen below. * write_func: - Make write_func throw an error on all error conditions. All callers of write_func were already checking for success and calling pg_fatal on all errors, so we might as well make the API support that case directly with simpler errorhandling as a result. * open_func: - zstd: move stream initialization from the open function to the read and write functions as they can have fatal errors. Also ensure to dup the file descriptor like none and gzip. - lz4: Ensure to dup the file descriptor like none and gzip. * close_func: - zstd: Ensure to close the file descriptor even if closing down the compressor fails, and clean up state allocation on fclose failures. Make sure to capture errors set by fclose. - lz4: Ensure to close the file descriptor even if closing down the compressor fails, and instead of calling pg_fatal log the failures using pg_log_error. Make sure to capture errors set by fclose. - none: Make sure to catch errors set by fclose. * read_func / gets_func: - Make read_func unconditionally return the number of read bytes instead of making it optional per implementation. - lz4: Make sure to call throw an error and not return -1 - gzip: gzread returning zero cannot be assumed to indicate EOF as it is documented to return zero for some types of errors. - lz4, zstd: Convert the _read_internal helper functions to not call pg_fatal on errors to be able to handle gets_func returning NULL on error. * getc_func: - zstd: Use an unsigned char rather than an int to read char into. * LZ4Stream_init: - Make sure to not switch to inited state until we know that initialization succeeded and reset errno just in case.On top of these changes there are minor comment cleanups andimprovements as well as an attempt to consistently reset errnoin codepaths where it is inspected.This work was initiated by a report of API misuse, which turnedinto a larger body of work. As this is an internal API thesechanges can be backpatched into all affected branches.Author: Tom Lane <tgl@sss.pgh.pa.us>Author: Daniel Gustafsson <daniel@yesql.se>Reported-by: Evgeniy Gorbanev <gorbanyoves@basealt.ru>Discussion:https://postgr.es/m/517794.1750082166@sss.pgh.pa.usBackpatch-through: 16
1 parent0c97c72 commit8980c72

File tree

8 files changed

+208
-165
lines changed

8 files changed

+208
-165
lines changed

‎src/bin/pg_dump/compress_gzip.c‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -251,34 +251,49 @@ InitCompressorGzip(CompressorState *cs,
251251
*----------------------
252252
*/
253253

254-
staticbool
255-
Gzip_read(void*ptr,size_tsize,size_t*rsize,CompressFileHandle*CFH)
254+
staticsize_t
255+
Gzip_read(void*ptr,size_tsize,CompressFileHandle*CFH)
256256
{
257257
gzFilegzfp= (gzFile)CFH->private_data;
258258
intgzret;
259259

260260
gzret=gzread(gzfp,ptr,size);
261-
if (gzret <=0&& !gzeof(gzfp))
261+
262+
/*
263+
* gzread returns zero on EOF as well as some error conditions, and less
264+
* than zero on other error conditions, so we need to inspect for EOF on
265+
* zero.
266+
*/
267+
if (gzret <=0)
262268
{
263269
interrnum;
264-
constchar*errmsg=gzerror(gzfp,&errnum);
270+
constchar*errmsg;
271+
272+
if (gzret==0&&gzeof(gzfp))
273+
return0;
274+
275+
errmsg=gzerror(gzfp,&errnum);
265276

266277
pg_fatal("could not read from input file: %s",
267278
errnum==Z_ERRNO ?strerror(errno) :errmsg);
268279
}
269280

270-
if (rsize)
271-
*rsize= (size_t)gzret;
272-
273-
return true;
281+
return (size_t)gzret;
274282
}
275283

276-
staticbool
284+
staticvoid
277285
Gzip_write(constvoid*ptr,size_tsize,CompressFileHandle*CFH)
278286
{
279287
gzFilegzfp= (gzFile)CFH->private_data;
288+
interrnum;
289+
constchar*errmsg;
280290

281-
returngzwrite(gzfp,ptr,size)>0;
291+
if (gzwrite(gzfp,ptr,size)!=size)
292+
{
293+
errmsg=gzerror(gzfp,&errnum);
294+
pg_fatal("could not write to file: %s",
295+
errnum==Z_ERRNO ?strerror(errno) :errmsg);
296+
}
282297
}
283298

284299
staticint

‎src/bin/pg_dump/compress_io.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ InitDiscoverCompressFileHandle(const char *path, const char *mode)
269269
}
270270

271271
CFH=InitCompressFileHandle(compression_spec);
272+
errno=0;
272273
if (!CFH->open_func(fname,-1,mode,CFH))
273274
{
274275
free_keep_errno(CFH);
@@ -289,6 +290,7 @@ EndCompressFileHandle(CompressFileHandle *CFH)
289290
{
290291
boolret= false;
291292

293+
errno=0;
292294
if (CFH->private_data)
293295
ret=CFH->close_func(CFH);
294296

‎src/bin/pg_dump/compress_io.h‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,21 +123,22 @@ struct CompressFileHandle
123123
CompressFileHandle*CFH);
124124

125125
/*
126-
* Read 'size' bytes of data from the file and store them into 'ptr'.
127-
*Optionally it will store the number of bytes read in 'rsize'.
126+
* Readup to'size' bytes of data from the file and store them into
127+
*'ptr'.
128128
*
129-
* Returns true on success and throws an internal error otherwise.
129+
* Returns number of bytes read (this might be less than 'size' if EOF was
130+
* reached). Exits via pg_fatal for all error conditions.
130131
*/
131-
bool(*read_func) (void*ptr,size_tsize,size_t*rsize,
132+
size_t(*read_func) (void*ptr,size_tsize,
132133
CompressFileHandle*CFH);
133134

134135
/*
135136
* Write 'size' bytes of data into the file from 'ptr'.
136137
*
137-
* Returnstrue on success and false on error.
138+
* Returnsnothing, exits via pg_fatal for all error conditions.
138139
*/
139-
bool(*write_func) (constvoid*ptr,size_tsize,
140-
structCompressFileHandle*CFH);
140+
void(*write_func) (constvoid*ptr,size_tsize,
141+
CompressFileHandle*CFH);
141142

142143
/*
143144
* Read at most size - 1 characters from the compress file handle into

‎src/bin/pg_dump/compress_lz4.c‎

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*-------------------------------------------------------------------------
1313
*/
1414
#include"postgres_fe.h"
15+
#include<unistd.h>
1516

1617
#include"compress_lz4.h"
1718
#include"pg_backup_utils.h"
@@ -358,7 +359,6 @@ LZ4Stream_init(LZ4State *state, int size, bool compressing)
358359
return true;
359360

360361
state->compressing=compressing;
361-
state->inited= true;
362362

363363
/* When compressing, write LZ4 header to the output stream. */
364364
if (state->compressing)
@@ -367,6 +367,7 @@ LZ4Stream_init(LZ4State *state, int size, bool compressing)
367367
if (!LZ4State_compression_init(state))
368368
return false;
369369

370+
errno=0;
370371
if (fwrite(state->buffer,1,state->compressedlen,state->fp)!=state->compressedlen)
371372
{
372373
errno= (errno) ?errno :ENOSPC;
@@ -390,6 +391,7 @@ LZ4Stream_init(LZ4State *state, int size, bool compressing)
390391
state->overflowlen=0;
391392
}
392393

394+
state->inited= true;
393395
return true;
394396
}
395397

@@ -457,7 +459,11 @@ LZ4Stream_read_internal(LZ4State *state, void *ptr, int ptrsize, bool eol_flag)
457459

458460
/* Lazy init */
459461
if (!LZ4Stream_init(state,size, false/* decompressing */ ))
462+
{
463+
pg_log_error("unable to initialize LZ4 library: %s",
464+
LZ4F_getErrorName(state->errcode));
460465
return-1;
466+
}
461467

462468
/* No work needs to be done for a zero-sized output buffer */
463469
if (size <=0)
@@ -484,7 +490,10 @@ LZ4Stream_read_internal(LZ4State *state, void *ptr, int ptrsize, bool eol_flag)
484490

485491
rsize=fread(readbuf,1,size,state->fp);
486492
if (rsize<size&& !feof(state->fp))
493+
{
494+
pg_log_error("could not read from input file: %m");
487495
return-1;
496+
}
488497

489498
rp= (char*)readbuf;
490499
rend= (char*)readbuf+rsize;
@@ -501,6 +510,8 @@ LZ4Stream_read_internal(LZ4State *state, void *ptr, int ptrsize, bool eol_flag)
501510
if (LZ4F_isError(status))
502511
{
503512
state->errcode=status;
513+
pg_log_error("could not read from input file: %s",
514+
LZ4F_getErrorName(state->errcode));
504515
return-1;
505516
}
506517

@@ -558,7 +569,7 @@ LZ4Stream_read_internal(LZ4State *state, void *ptr, int ptrsize, bool eol_flag)
558569
/*
559570
* Compress size bytes from ptr and write them to the stream.
560571
*/
561-
staticbool
572+
staticvoid
562573
LZ4Stream_write(constvoid*ptr,size_tsize,CompressFileHandle*CFH)
563574
{
564575
LZ4State*state= (LZ4State*)CFH->private_data;
@@ -567,7 +578,8 @@ LZ4Stream_write(const void *ptr, size_t size, CompressFileHandle *CFH)
567578

568579
/* Lazy init */
569580
if (!LZ4Stream_init(state,size, true))
570-
return false;
581+
pg_fatal("unable to initialize LZ4 library: %s",
582+
LZ4F_getErrorName(state->errcode));
571583

572584
while (remaining>0)
573585
{
@@ -578,39 +590,32 @@ LZ4Stream_write(const void *ptr, size_t size, CompressFileHandle *CFH)
578590
status=LZ4F_compressUpdate(state->ctx,state->buffer,state->buflen,
579591
ptr,chunk,NULL);
580592
if (LZ4F_isError(status))
581-
{
582-
state->errcode=status;
583-
return false;
584-
}
593+
pg_fatal("error during writing: %s",LZ4F_getErrorName(status));
585594

595+
errno=0;
586596
if (fwrite(state->buffer,1,status,state->fp)!=status)
587597
{
588598
errno= (errno) ?errno :ENOSPC;
589-
return false;
599+
pg_fatal("error during writing: %m");
590600
}
591601

592602
ptr= ((constchar*)ptr)+chunk;
593603
}
594-
595-
return true;
596604
}
597605

598606
/*
599607
* fread() equivalent implementation for LZ4 compressed files.
600608
*/
601-
staticbool
602-
LZ4Stream_read(void*ptr,size_tsize,size_t*rsize,CompressFileHandle*CFH)
609+
staticsize_t
610+
LZ4Stream_read(void*ptr,size_tsize,CompressFileHandle*CFH)
603611
{
604612
LZ4State*state= (LZ4State*)CFH->private_data;
605613
intret;
606614

607615
if ((ret=LZ4Stream_read_internal(state,ptr,size, false))<0)
608616
pg_fatal("could not read from input file: %s",LZ4Stream_get_error(CFH));
609617

610-
if (rsize)
611-
*rsize= (size_t)ret;
612-
613-
return true;
618+
return (size_t)ret;
614619
}
615620

616621
/*
@@ -643,11 +648,13 @@ LZ4Stream_gets(char *ptr, int size, CompressFileHandle *CFH)
643648
intret;
644649

645650
ret=LZ4Stream_read_internal(state,ptr,size-1, true);
646-
if (ret<0|| (ret==0&& !LZ4Stream_eof(CFH)))
647-
pg_fatal("could not read from input file: %s",LZ4Stream_get_error(CFH));
648651

649-
/* Done reading */
650-
if (ret==0)
652+
/*
653+
* LZ4Stream_read_internal returning 0 or -1 means that it was either an
654+
* EOF or an error, but gets_func is defined to return NULL in either case
655+
* so we can treat both the same here.
656+
*/
657+
if (ret <=0)
651658
returnNULL;
652659

653660
/*
@@ -669,6 +676,7 @@ LZ4Stream_close(CompressFileHandle *CFH)
669676
FILE*fp;
670677
LZ4State*state= (LZ4State*)CFH->private_data;
671678
size_tstatus;
679+
intret;
672680

673681
fp=state->fp;
674682
if (state->inited)
@@ -677,55 +685,67 @@ LZ4Stream_close(CompressFileHandle *CFH)
677685
{
678686
status=LZ4F_compressEnd(state->ctx,state->buffer,state->buflen,NULL);
679687
if (LZ4F_isError(status))
680-
pg_fatal("could not end compression: %s",
681-
LZ4F_getErrorName(status));
682-
elseif (fwrite(state->buffer,1,status,state->fp)!=status)
683688
{
684-
errno= (errno) ?errno :ENOSPC;
685-
WRITE_ERROR_EXIT;
689+
pg_log_error("could not end compression: %s",
690+
LZ4F_getErrorName(status));
691+
}
692+
else
693+
{
694+
errno=0;
695+
if (fwrite(state->buffer,1,status,state->fp)!=status)
696+
{
697+
errno= (errno) ?errno :ENOSPC;
698+
pg_log_error("could not write to output file: %m");
699+
}
686700
}
687701

688702
status=LZ4F_freeCompressionContext(state->ctx);
689703
if (LZ4F_isError(status))
690-
pg_fatal("could not end compression: %s",
691-
LZ4F_getErrorName(status));
704+
pg_log_error("could not end compression: %s",
705+
LZ4F_getErrorName(status));
692706
}
693707
else
694708
{
695709
status=LZ4F_freeDecompressionContext(state->dtx);
696710
if (LZ4F_isError(status))
697-
pg_fatal("could not end decompression: %s",
698-
LZ4F_getErrorName(status));
711+
pg_log_error("could not end decompression: %s",
712+
LZ4F_getErrorName(status));
699713
pg_free(state->overflowbuf);
700714
}
701715

702716
pg_free(state->buffer);
703717
}
704718

705719
pg_free(state);
720+
CFH->private_data=NULL;
706721

707-
returnfclose(fp)==0;
722+
errno=0;
723+
ret=fclose(fp);
724+
if (ret!=0)
725+
{
726+
pg_log_error("could not close file: %m");
727+
return false;
728+
}
729+
730+
return true;
708731
}
709732

710733
staticbool
711734
LZ4Stream_open(constchar*path,intfd,constchar*mode,
712735
CompressFileHandle*CFH)
713736
{
714-
FILE*fp;
715737
LZ4State*state= (LZ4State*)CFH->private_data;
716738

717739
if (fd >=0)
718-
fp=fdopen(fd,mode);
740+
state->fp=fdopen(dup(fd),mode);
719741
else
720-
fp=fopen(path,mode);
721-
if (fp==NULL)
742+
state->fp=fopen(path,mode);
743+
if (state->fp==NULL)
722744
{
723745
state->errcode=errno;
724746
return false;
725747
}
726748

727-
state->fp=fp;
728-
729749
return true;
730750
}
731751

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp