Skip to content

Commit 0c5803b

Browse files
peterepetere
committed
Refactor new file permission handling
The file handling functions from fd.c were called with a diverse mix of notations for the file permissions when they were opening new files. Almost all files created by the server should have the same permissions set. So change the API so that e.g. OpenTransientFile() automatically uses the standard permissions set, and OpenTransientFilePerm() is a new function that takes an explicit permissions set for the few cases where it is needed. This also saves an unnecessary argument for call sites that are just opening an existing file. While we're reviewing these APIs, get rid of the FileName typedef and use the standard const char * for the file name and mode_t for the file mode. This makes these functions match other file handling functions and removes an unnecessary layer of mysteriousness. We can also get rid of a few casts that way. Author: David Steele <david@pgmasters.net>
1 parent 404ba54 commit 0c5803b

File tree

21 files changed

+110
-102
lines changed

21 files changed

+110
-102
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,8 +1869,7 @@ qtext_store(const char *query, int query_len,
18691869
*query_offset = off;
18701870

18711871
/* Now write the data into the successfully-reserved part of the file */
1872-
fd = OpenTransientFile(PGSS_TEXT_FILE, O_RDWR | O_CREAT | PG_BINARY,
1873-
S_IRUSR | S_IWUSR);
1872+
fd = OpenTransientFile(PGSS_TEXT_FILE, O_RDWR | O_CREAT | PG_BINARY);
18741873
if (fd < 0)
18751874
goto error;
18761875

@@ -1934,7 +1933,7 @@ qtext_load_file(Size *buffer_size)
19341933
int fd;
19351934
struct stat stat;
19361935

1937-
fd = OpenTransientFile(PGSS_TEXT_FILE, O_RDONLY | PG_BINARY, 0);
1936+
fd = OpenTransientFile(PGSS_TEXT_FILE, O_RDONLY | PG_BINARY);
19381937
if (fd < 0)
19391938
{
19401939
if (errno != ENOENT)

‎src/backend/access/heap/rewriteheap.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,8 +1013,7 @@ logical_rewrite_log_mapping(RewriteState state, TransactionId xid,
10131013
src->off = 0;
10141014
memcpy(src->path, path, sizeof(path));
10151015
src->vfd = PathNameOpenFile(path,
1016-
O_CREAT | O_EXCL | O_WRONLY | PG_BINARY,
1017-
S_IRUSR | S_IWUSR);
1016+
O_CREAT | O_EXCL | O_WRONLY | PG_BINARY);
10181017
if (src->vfd < 0)
10191018
ereport(ERROR,
10201019
(errcode_for_file_access(),
@@ -1133,8 +1132,7 @@ heap_xlog_logical_rewrite(XLogReaderState *r)
11331132
xlrec->mapped_xid, XLogRecGetXid(r));
11341133

11351134
fd = OpenTransientFile(path,
1136-
O_CREAT | O_WRONLY | PG_BINARY,
1137-
S_IRUSR | S_IWUSR);
1135+
O_CREAT | O_WRONLY | PG_BINARY);
11381136
if (fd < 0)
11391137
ereport(ERROR,
11401138
(errcode_for_file_access(),
@@ -1258,7 +1256,7 @@ CheckPointLogicalRewriteHeap(void)
12581256
}
12591257
else
12601258
{
1261-
int fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
1259+
int fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
12621260

12631261
/*
12641262
* The file cannot vanish due to concurrency since this function

‎src/backend/access/transam/slru.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int pageno)
599599

600600
SlruFileName(ctl, path, segno);
601601

602-
fd = OpenTransientFile(path, O_RDWR | PG_BINARY, S_IRUSR | S_IWUSR);
602+
fd = OpenTransientFile(path, O_RDWR | PG_BINARY);
603603
if (fd < 0)
604604
{
605605
/* expected: file doesn't exist */
@@ -654,7 +654,7 @@ SlruPhysicalReadPage(SlruCtl ctl, int pageno, int slotno)
654654
* SlruPhysicalWritePage). Hence, if we are InRecovery, allow the case
655655
* where the file doesn't exist, and return zeroes instead.
656656
*/
657-
fd = OpenTransientFile(path, O_RDWR | PG_BINARY, S_IRUSR | S_IWUSR);
657+
fd = OpenTransientFile(path, O_RDWR | PG_BINARY);
658658
if (fd < 0)
659659
{
660660
if (errno != ENOENT || !InRecovery)
@@ -804,8 +804,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruFlush fdata)
804804
* don't use O_EXCL or O_TRUNC or anything like that.
805805
*/
806806
SlruFileName(ctl, path, segno);
807-
fd = OpenTransientFile(path, O_RDWR | O_CREAT | PG_BINARY,
808-
S_IRUSR | S_IWUSR);
807+
fd = OpenTransientFile(path, O_RDWR | O_CREAT | PG_BINARY);
809808
if (fd < 0)
810809
{
811810
slru_errcause = SLRU_OPEN_FAILED;

‎src/backend/access/transam/timeline.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
307307
unlink(tmppath);
308308

309309
/* do not use get_sync_bit() here --- want to fsync only at end of fill */
310-
fd = OpenTransientFile(tmppath, O_RDWR | O_CREAT | O_EXCL,
311-
S_IRUSR | S_IWUSR);
310+
fd = OpenTransientFile(tmppath, O_RDWR | O_CREAT | O_EXCL);
312311
if (fd < 0)
313312
ereport(ERROR,
314313
(errcode_for_file_access(),
@@ -325,7 +324,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
325324
else
326325
TLHistoryFilePath(path, parentTLI);
327326

328-
srcfd = OpenTransientFile(path, O_RDONLY, 0);
327+
srcfd = OpenTransientFile(path, O_RDONLY);
329328
if (srcfd < 0)
330329
{
331330
if (errno != ENOENT)
@@ -459,8 +458,7 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int size)
459458
unlink(tmppath);
460459

461460
/* do not use get_sync_bit() here --- want to fsync only at end of fill */
462-
fd = OpenTransientFile(tmppath, O_RDWR | O_CREAT | O_EXCL,
463-
S_IRUSR | S_IWUSR);
461+
fd = OpenTransientFile(tmppath, O_RDWR | O_CREAT | O_EXCL);
464462
if (fd < 0)
465463
ereport(ERROR,
466464
(errcode_for_file_access(),

‎src/backend/access/transam/twophase.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,7 +1195,7 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
11951195

11961196
TwoPhaseFilePath(path, xid);
11971197

1198-
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
1198+
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
11991199
if (fd < 0)
12001200
{
12011201
if (give_warnings)
@@ -1581,8 +1581,7 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
15811581
TwoPhaseFilePath(path, xid);
15821582

15831583
fd = OpenTransientFile(path,
1584-
O_CREAT | O_TRUNC | O_WRONLY | PG_BINARY,
1585-
S_IRUSR | S_IWUSR);
1584+
O_CREAT | O_TRUNC | O_WRONLY | PG_BINARY);
15861585
if (fd < 0)
15871586
ereport(ERROR,
15881587
(errcode_for_file_access(),

‎src/backend/access/transam/xlog.c

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3185,8 +3185,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
31853185
*/
31863186
if (*use_existent)
31873187
{
3188-
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method),
3189-
S_IRUSR | S_IWUSR);
3188+
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method));
31903189
if (fd < 0)
31913190
{
31923191
if (errno != ENOENT)
@@ -3211,8 +3210,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
32113210
unlink(tmppath);
32123211

32133212
/* do not use get_sync_bit() here --- want to fsync only at end of fill */
3214-
fd = BasicOpenFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
3215-
S_IRUSR | S_IWUSR);
3213+
fd = BasicOpenFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY);
32163214
if (fd < 0)
32173215
ereport(ERROR,
32183216
(errcode_for_file_access(),
@@ -3308,8 +3306,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
33083306
*use_existent = false;
33093307

33103308
/* Now open original target segment (might not be file I just made) */
3311-
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method),
3312-
S_IRUSR | S_IWUSR);
3309+
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method));
33133310
if (fd < 0)
33143311
ereport(ERROR,
33153312
(errcode_for_file_access(),
@@ -3350,7 +3347,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
33503347
* Open the source file
33513348
*/
33523349
XLogFilePath(path, srcTLI, srcsegno, wal_segment_size);
3353-
srcfd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
3350+
srcfd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
33543351
if (srcfd < 0)
33553352
ereport(ERROR,
33563353
(errcode_for_file_access(),
@@ -3364,8 +3361,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
33643361
unlink(tmppath);
33653362

33663363
/* do not use get_sync_bit() here --- want to fsync only at end of fill */
3367-
fd = OpenTransientFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
3368-
S_IRUSR | S_IWUSR);
3364+
fd = OpenTransientFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY);
33693365
if (fd < 0)
33703366
ereport(ERROR,
33713367
(errcode_for_file_access(),
@@ -3543,8 +3539,7 @@ XLogFileOpen(XLogSegNo segno)
35433539

35443540
XLogFilePath(path, ThisTimeLineID, segno, wal_segment_size);
35453541

3546-
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method),
3547-
S_IRUSR | S_IWUSR);
3542+
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method));
35483543
if (fd < 0)
35493544
ereport(PANIC,
35503545
(errcode_for_file_access(),
@@ -3610,7 +3605,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
36103605
snprintf(path, MAXPGPATH, XLOGDIR "/%s", xlogfname);
36113606
}
36123607

3613-
fd = BasicOpenFile(path, O_RDONLY | PG_BINARY, 0);
3608+
fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
36143609
if (fd >= 0)
36153610
{
36163611
/* Success! */
@@ -4449,8 +4444,7 @@ WriteControlFile(void)
44494444
memcpy(buffer, ControlFile, sizeof(ControlFileData));
44504445

44514446
fd = BasicOpenFile(XLOG_CONTROL_FILE,
4452-
O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
4453-
S_IRUSR | S_IWUSR);
4447+
O_RDWR | O_CREAT | O_EXCL | PG_BINARY);
44544448
if (fd < 0)
44554449
ereport(PANIC,
44564450
(errcode_for_file_access(),
@@ -4494,8 +4488,7 @@ ReadControlFile(void)
44944488
* Read data...
44954489
*/
44964490
fd = BasicOpenFile(XLOG_CONTROL_FILE,
4497-
O_RDWR | PG_BINARY,
4498-
S_IRUSR | S_IWUSR);
4491+
O_RDWR | PG_BINARY);
44994492
if (fd < 0)
45004493
ereport(PANIC,
45014494
(errcode_for_file_access(),
@@ -4695,8 +4688,7 @@ UpdateControlFile(void)
46954688
FIN_CRC32C(ControlFile->crc);
46964689

46974690
fd = BasicOpenFile(XLOG_CONTROL_FILE,
4698-
O_RDWR | PG_BINARY,
4699-
S_IRUSR | S_IWUSR);
4691+
O_RDWR | PG_BINARY);
47004692
if (fd < 0)
47014693
ereport(PANIC,
47024694
(errcode_for_file_access(),

‎src/backend/access/transam/xlogutils.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,7 @@ XLogRead(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
694694

695695
XLogFilePath(path, tli, sendSegNo, segsize);
696696

697-
sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY, 0);
697+
sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
698698

699699
if (sendFile < 0)
700700
{

‎src/backend/catalog/catalog.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence)
444444

445445
/* Check for existing file of same name */
446446
rpath = relpath(rnode, MAIN_FORKNUM);
447-
fd = BasicOpenFile(rpath, O_RDONLY | PG_BINARY, 0);
447+
fd = BasicOpenFile(rpath, O_RDONLY | PG_BINARY);
448448

449449
if (fd >= 0)
450450
{

‎src/backend/libpq/be-fsstubs.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ lo_import_internal(text *filename, Oid lobjOid)
462462
* open the file to be read in
463463
*/
464464
text_to_cstring_buffer(filename, fnamebuf, sizeof(fnamebuf));
465-
fd = OpenTransientFile(fnamebuf, O_RDONLY | PG_BINARY, S_IRWXU);
465+
fd = OpenTransientFile(fnamebuf, O_RDONLY | PG_BINARY);
466466
if (fd < 0)
467467
ereport(ERROR,
468468
(errcode_for_file_access(),
@@ -540,8 +540,8 @@ be_lo_export(PG_FUNCTION_ARGS)
540540
oumask = umask(S_IWGRP | S_IWOTH);
541541
PG_TRY();
542542
{
543-
fd = OpenTransientFile(fnamebuf, O_CREAT | O_WRONLY | O_TRUNC | PG_BINARY,
544-
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
543+
fd = OpenTransientFilePerm(fnamebuf, O_CREAT | O_WRONLY | O_TRUNC | PG_BINARY,
544+
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
545545
}
546546
PG_CATCH();
547547
{

‎src/backend/replication/logical/origin.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -546,9 +546,8 @@ CheckPointReplicationOrigin(void)
546546
* no other backend can perform this at the same time, we're protected by
547547
* CheckpointLock.
548548
*/
549-
tmpfd = OpenTransientFile((char *) tmppath,
550-
O_CREAT | O_EXCL | O_WRONLY | PG_BINARY,
551-
S_IRUSR | S_IWUSR);
549+
tmpfd = OpenTransientFile(tmppath,
550+
O_CREAT | O_EXCL | O_WRONLY | PG_BINARY);
552551
if (tmpfd < 0)
553552
ereport(PANIC,
554553
(errcode_for_file_access(),
@@ -660,7 +659,7 @@ StartupReplicationOrigin(void)
660659

661660
elog(DEBUG2, "starting up replication origin progress state");
662661

663-
fd = OpenTransientFile((char *) path, O_RDONLY | PG_BINARY, 0);
662+
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
664663

665664
/*
666665
* might have had max_replication_slots == 0 last run, or we just brought

‎src/backend/replication/logical/reorderbuffer.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,8 +2104,7 @@ ReorderBufferSerializeTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
21042104

21052105
/* open segment, create it if necessary */
21062106
fd = OpenTransientFile(path,
2107-
O_CREAT | O_WRONLY | O_APPEND | PG_BINARY,
2108-
S_IRUSR | S_IWUSR);
2107+
O_CREAT | O_WRONLY | O_APPEND | PG_BINARY);
21092108

21102109
if (fd < 0)
21112110
ereport(ERROR,
@@ -2349,7 +2348,7 @@ ReorderBufferRestoreChanges(ReorderBuffer *rb, ReorderBufferTXN *txn,
23492348
NameStr(MyReplicationSlot->data.name), txn->xid,
23502349
(uint32) (recptr >> 32), (uint32) recptr);
23512350

2352-
*fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
2351+
*fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
23532352
if (*fd < 0 && errno == ENOENT)
23542353
{
23552354
*fd = -1;
@@ -3038,7 +3037,7 @@ ApplyLogicalMappingFile(HTAB *tuplecid_data, Oid relid, const char *fname)
30383037
LogicalRewriteMappingData map;
30393038

30403039
sprintf(path, "pg_logical/mappings/%s", fname);
3041-
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
3040+
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
30423041
if (fd < 0)
30433042
ereport(ERROR,
30443043
(errcode_for_file_access(),

‎src/backend/replication/logical/snapbuild.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,8 +1597,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
15971597

15981598
/* we have valid data now, open tempfile and write it there */
15991599
fd = OpenTransientFile(tmppath,
1600-
O_CREAT | O_EXCL | O_WRONLY | PG_BINARY,
1601-
S_IRUSR | S_IWUSR);
1600+
O_CREAT | O_EXCL | O_WRONLY | PG_BINARY);
16021601
if (fd < 0)
16031602
ereport(ERROR,
16041603
(errmsg("could not open file \"%s\": %m", path)));
@@ -1682,7 +1681,7 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
16821681
sprintf(path, "pg_logical/snapshots/%X-%X.snap",
16831682
(uint32) (lsn >> 32), (uint32) lsn);
16841683

1685-
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
1684+
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
16861685

16871686
if (fd < 0 && errno == ENOENT)
16881687
return false;

‎src/backend/replication/slot.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,9 +1233,7 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
12331233
sprintf(tmppath, "%s/state.tmp", dir);
12341234
sprintf(path, "%s/state", dir);
12351235

1236-
fd = OpenTransientFile(tmppath,
1237-
O_CREAT | O_EXCL | O_WRONLY | PG_BINARY,
1238-
S_IRUSR | S_IWUSR);
1236+
fd = OpenTransientFile(tmppath, O_CREAT | O_EXCL | O_WRONLY | PG_BINARY);
12391237
if (fd < 0)
12401238
{
12411239
ereport(elevel,
@@ -1354,7 +1352,7 @@ RestoreSlotFromDisk(const char *name)
13541352

13551353
elog(DEBUG1, "restoring replication slot from \"%s\"", path);
13561354

1357-
fd = OpenTransientFile(path, O_RDWR | PG_BINARY, 0);
1355+
fd = OpenTransientFile(path, O_RDWR | PG_BINARY);
13581356

13591357
/*
13601358
* We do not need to handle this as we are rename()ing the directory into

‎src/backend/replication/walsender.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
472472
pq_sendint(&buf, len, 4); /* col1 len */
473473
pq_sendbytes(&buf, histfname, len);
474474

475-
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0666);
475+
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
476476
if (fd < 0)
477477
ereport(ERROR,
478478
(errcode_for_file_access(),
@@ -2366,7 +2366,7 @@ XLogRead(char *buf, XLogRecPtr startptr, Size count)
23662366

23672367
XLogFilePath(path, curFileTimeLine, sendSegNo, wal_segment_size);
23682368

2369-
sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY, 0);
2369+
sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
23702370
if (sendFile < 0)
23712371
{
23722372
/*

‎src/backend/storage/file/copydir.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,13 @@ copy_file(char *fromfile, char *tofile)
148148
/*
149149
* Open the files
150150
*/
151-
srcfd = OpenTransientFile(fromfile, O_RDONLY | PG_BINARY, 0);
151+
srcfd = OpenTransientFile(fromfile, O_RDONLY | PG_BINARY);
152152
if (srcfd < 0)
153153
ereport(ERROR,
154154
(errcode_for_file_access(),
155155
errmsg("could not open file \"%s\": %m", fromfile)));
156156

157-
dstfd = OpenTransientFile(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
158-
S_IRUSR | S_IWUSR);
157+
dstfd = OpenTransientFile(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY);
159158
if (dstfd < 0)
160159
ereport(ERROR,
161160
(errcode_for_file_access(),

0 commit comments

Comments
 (0)