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

Commitd3b5775

Browse files
committed
Improve type handling in pg_dump's compress file API
After0da243f got committed, we've received a report about a compilerwarning, related to the new LZ4File_gets() function: compress_lz4.c: In function 'LZ4File_gets': compress_lz4.c:492:19: warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits] 492 | if (dsize < 0)The reason is very simple - dsize is declared as size_t, which is anunsigned integer, and thus the check is pointless and we might fail tonotice an error in some cases (or fail in a strange way a bit later).The warning could have been silenced by simply changing the type, but werealized the API mostly assumes all the libraries use the same types andreport errors the same way (e.g. by returning 0 and/or negative value).But we can't make this assumption - the gzip/lz4 libraries alreadydisagree on some of this, and even if they did a library added in thefuture might not.The right solution is to define what the API does, and translate thelibrary-specific behavior in consistent way (so that the internal errorsare not exposed to users of our compression API). So this adjusts thedata types in a couple places, so that we don't miss library errors, andsimplifies and unifies the error reporting to simply return true/false(instead of e.g. size_t).While at it, make sure LZ4File_open_write() does not clobber errno incase open_func() fails.Author: Georgios KokolatosReported-by: Alexander LakhinReviewed-by: Tomas Vondra, Justin PryzbyDiscussion:https://postgr.es/m/33496f7c-3449-1426-d568-63f6bca2ac1f@gmail.com
1 parenta326aac commitd3b5775

File tree

7 files changed

+148
-115
lines changed

7 files changed

+148
-115
lines changed

‎src/bin/pg_dump/compress_gzip.c

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -233,14 +233,14 @@ InitCompressorGzip(CompressorState *cs,
233233
*----------------------
234234
*/
235235

236-
staticsize_t
237-
Gzip_read(void*ptr,size_tsize,CompressFileHandle*CFH)
236+
staticbool
237+
Gzip_read(void*ptr,size_tsize,size_t*rsize,CompressFileHandle*CFH)
238238
{
239239
gzFilegzfp= (gzFile)CFH->private_data;
240-
size_tret;
240+
intgzret;
241241

242-
ret=gzread(gzfp,ptr,size);
243-
if (ret!=size&& !gzeof(gzfp))
242+
gzret=gzread(gzfp,ptr,size);
243+
if (gzret <=0&& !gzeof(gzfp))
244244
{
245245
interrnum;
246246
constchar*errmsg=gzerror(gzfp,&errnum);
@@ -249,15 +249,18 @@ Gzip_read(void *ptr, size_t size, CompressFileHandle *CFH)
249249
errnum==Z_ERRNO ?strerror(errno) :errmsg);
250250
}
251251

252-
returnret;
252+
if (rsize)
253+
*rsize= (size_t)gzret;
254+
255+
return true;
253256
}
254257

255-
staticsize_t
258+
staticbool
256259
Gzip_write(constvoid*ptr,size_tsize,CompressFileHandle*CFH)
257260
{
258261
gzFilegzfp= (gzFile)CFH->private_data;
259262

260-
returngzwrite(gzfp,ptr,size);
263+
returngzwrite(gzfp,ptr,size)>0;
261264
}
262265

263266
staticint
@@ -287,22 +290,22 @@ Gzip_gets(char *ptr, int size, CompressFileHandle *CFH)
287290
returngzgets(gzfp,ptr,size);
288291
}
289292

290-
staticint
293+
staticbool
291294
Gzip_close(CompressFileHandle*CFH)
292295
{
293296
gzFilegzfp= (gzFile)CFH->private_data;
294297

295298
CFH->private_data=NULL;
296299

297-
returngzclose(gzfp);
300+
returngzclose(gzfp)==Z_OK;
298301
}
299302

300-
staticint
303+
staticbool
301304
Gzip_eof(CompressFileHandle*CFH)
302305
{
303306
gzFilegzfp= (gzFile)CFH->private_data;
304307

305-
returngzeof(gzfp);
308+
returngzeof(gzfp)==1;
306309
}
307310

308311
staticconstchar*
@@ -319,7 +322,7 @@ Gzip_get_error(CompressFileHandle *CFH)
319322
returnerrmsg;
320323
}
321324

322-
staticint
325+
staticbool
323326
Gzip_open(constchar*path,intfd,constchar*mode,CompressFileHandle*CFH)
324327
{
325328
gzFilegzfp;
@@ -342,18 +345,18 @@ Gzip_open(const char *path, int fd, const char *mode, CompressFileHandle *CFH)
342345
gzfp=gzopen(path,mode_compression);
343346

344347
if (gzfp==NULL)
345-
return1;
348+
returnfalse;
346349

347350
CFH->private_data=gzfp;
348351

349-
return0;
352+
returntrue;
350353
}
351354

352-
staticint
355+
staticbool
353356
Gzip_open_write(constchar*path,constchar*mode,CompressFileHandle*CFH)
354357
{
355358
char*fname;
356-
intret;
359+
boolret;
357360
intsave_errno;
358361

359362
fname=psprintf("%s.gz",path);

‎src/bin/pg_dump/compress_io.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ InitDiscoverCompressFileHandle(const char *path, const char *mode)
262262
}
263263

264264
CFH=InitCompressFileHandle(compression_spec);
265-
if (CFH->open_func(fname,-1,mode,CFH))
265+
if (!CFH->open_func(fname,-1,mode,CFH))
266266
{
267267
free_keep_errno(CFH);
268268
CFH=NULL;
@@ -275,12 +275,12 @@ InitDiscoverCompressFileHandle(const char *path, const char *mode)
275275
/*
276276
* Close an open file handle and release its memory.
277277
*
278-
* On failure, returnsan error value and sets errno appropriately.
278+
* On failure, returnsfalse and sets errno appropriately.
279279
*/
280-
int
280+
bool
281281
EndCompressFileHandle(CompressFileHandle*CFH)
282282
{
283-
intret=0;
283+
boolret=false;
284284

285285
if (CFH->private_data)
286286
ret=CFH->close_func(CFH);

‎src/bin/pg_dump/compress_io.h

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,28 +100,38 @@ struct CompressFileHandle
100100
* Pass either 'path' or 'fd' depending on whether a file path or a file
101101
* descriptor is available. 'mode' can be one of 'r', 'rb', 'w', 'wb',
102102
* 'a', and 'ab'. Requires an already initialized CompressFileHandle.
103+
*
104+
* Returns true on success and false on error.
103105
*/
104-
int(*open_func) (constchar*path,intfd,constchar*mode,
106+
bool(*open_func) (constchar*path,intfd,constchar*mode,
105107
CompressFileHandle*CFH);
106108

107109
/*
108110
* Open a file for writing.
109111
*
110112
* 'mode' can be one of 'w', 'wb', 'a', and 'ab'. Requires an already
111113
* initialized CompressFileHandle.
114+
*
115+
* Returns true on success and false on error.
112116
*/
113-
int(*open_write_func) (constchar*path,constchar*mode,
117+
bool(*open_write_func) (constchar*path,constchar*mode,
114118
CompressFileHandle*CFH);
115119

116120
/*
117121
* Read 'size' bytes of data from the file and store them into 'ptr'.
122+
* Optionally it will store the number of bytes read in 'rsize'.
123+
*
124+
* Returns true on success and throws an internal error otherwise.
118125
*/
119-
size_t(*read_func) (void*ptr,size_tsize,CompressFileHandle*CFH);
126+
bool(*read_func) (void*ptr,size_tsize,size_t*rsize,
127+
CompressFileHandle*CFH);
120128

121129
/*
122130
* Write 'size' bytes of data into the file from 'ptr'.
131+
*
132+
* Returns true on success and false on error.
123133
*/
124-
size_t(*write_func) (constvoid*ptr,size_tsize,
134+
bool(*write_func) (constvoid*ptr,size_tsize,
125135
structCompressFileHandle*CFH);
126136

127137
/*
@@ -130,28 +140,38 @@ struct CompressFileHandle
130140
*
131141
* Stop if an EOF or a newline is found first. 's' is always null
132142
* terminated and contains the newline if it was found.
143+
*
144+
* Returns 's' on success, and NULL on error or when end of file occurs
145+
* while no characters have been read.
133146
*/
134147
char*(*gets_func) (char*s,intsize,CompressFileHandle*CFH);
135148

136149
/*
137150
* Read the next character from the compress file handle as 'unsigned
138151
* char' cast into 'int'.
152+
*
153+
* Returns the character read on success and throws an internal error
154+
* otherwise. It treats EOF as error.
139155
*/
140156
int(*getc_func) (CompressFileHandle*CFH);
141157

142158
/*
143159
* Test if EOF is reached in the compress file handle.
160+
*
161+
* Returns true if it is reached.
144162
*/
145-
int(*eof_func) (CompressFileHandle*CFH);
163+
bool(*eof_func) (CompressFileHandle*CFH);
146164

147165
/*
148166
* Close an open file handle.
167+
*
168+
* Returns true on success and false on error.
149169
*/
150-
int(*close_func) (CompressFileHandle*CFH);
170+
bool(*close_func) (CompressFileHandle*CFH);
151171

152172
/*
153-
* Get a pointer to a string that describes an error that occurred during a
154-
* compress file handle operation.
173+
* Get a pointer to a string that describes an error that occurred during
174+
*acompress file handle operation.
155175
*/
156176
constchar*(*get_error_func) (CompressFileHandle*CFH);
157177

@@ -178,5 +198,5 @@ extern CompressFileHandle *InitCompressFileHandle(const pg_compress_specificatio
178198
*/
179199
externCompressFileHandle*InitDiscoverCompressFileHandle(constchar*path,
180200
constchar*mode);
181-
externintEndCompressFileHandle(CompressFileHandle*CFH);
201+
externboolEndCompressFileHandle(CompressFileHandle*CFH);
182202
#endif

0 commit comments

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