Skip to content

Commit 1c42844

Browse files
felixhandtefelixhandte
authored
Merge pull request #3479 from felixhandte/faster-file-ops
Use `f`-variants of `chmod()` and `chown()`
2 parents 3c50854 + f746c37 commit 1c42844

11 files changed

+152
-64
lines changed

‎programs/fileio.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1681,6 +1681,7 @@ static int FIO_compressFilename_dstFile(FIO_ctx_t* const fCtx,
16811681
int result;
16821682
int transferStat = 0;
16831683
FILE *dstFile;
1684+
int dstFd = -1;
16841685

16851686
assert(AIO_ReadPool_getFile(ress.readCtx) != NULL);
16861687
if (AIO_WritePool_getFile(ress.writeCtx) == NULL) {
@@ -1696,6 +1697,7 @@ static int FIO_compressFilename_dstFile(FIO_ctx_t* const fCtx,
16961697
DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: opening dst: %s \n", dstFileName);
16971698
dstFile = FIO_openDstFile(fCtx, prefs, srcFileName, dstFileName, dstFileInitialPermissions);
16981699
if (dstFile==NULL) return 1; /* could not open dstFileName */
1700+
dstFd = fileno(dstFile);
16991701
AIO_WritePool_setFile(ress.writeCtx, dstFile);
17001702
/* Must only be added after FIO_openDstFile() succeeds.
17011703
* Otherwise we may delete the destination file if it already exists,
@@ -1709,14 +1711,20 @@ static int FIO_compressFilename_dstFile(FIO_ctx_t* const fCtx,
17091711
if (closeDstFile) {
17101712
clearHandler();
17111713

1714+
if (transferStat) {
1715+
UTIL_setFDStat(dstFd, dstFileName, srcFileStat);
1716+
}
1717+
17121718
DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: closing dst: %s \n", dstFileName);
17131719
if (AIO_WritePool_closeFile(ress.writeCtx)) { /* error closing file */
17141720
DISPLAYLEVEL(1, "zstd: %s: %s \n", dstFileName, strerror(errno));
17151721
result=1;
17161722
}
1723+
17171724
if (transferStat) {
1718-
UTIL_setFileStat(dstFileName, srcFileStat);
1725+
UTIL_utime(dstFileName, srcFileStat);
17191726
}
1727+
17201728
if ( (result != 0) /* operation failure */
17211729
&& strcmp(dstFileName, stdoutmark) /* special case : don't remove() stdout */
17221730
) {
@@ -2540,6 +2548,7 @@ static int FIO_decompressDstFile(FIO_ctx_t* const fCtx,
25402548
int result;
25412549
int releaseDstFile = 0;
25422550
int transferStat = 0;
2551+
int dstFd = 0;
25432552

25442553
if ((AIO_WritePool_getFile(ress.writeCtx) == NULL) && (prefs->testMode == 0)) {
25452554
FILE *dstFile;
@@ -2555,6 +2564,7 @@ static int FIO_decompressDstFile(FIO_ctx_t* const fCtx,
25552564

25562565
dstFile = FIO_openDstFile(fCtx, prefs, srcFileName, dstFileName, dstFilePermissions);
25572566
if (dstFile==NULL) return 1;
2567+
dstFd = fileno(dstFile);
25582568
AIO_WritePool_setFile(ress.writeCtx, dstFile);
25592569

25602570
/* Must only be added after FIO_openDstFile() succeeds.
@@ -2568,13 +2578,18 @@ static int FIO_decompressDstFile(FIO_ctx_t* const fCtx,
25682578

25692579
if (releaseDstFile) {
25702580
clearHandler();
2581+
2582+
if (transferStat) {
2583+
UTIL_setFDStat(dstFd, dstFileName, srcFileStat);
2584+
}
2585+
25712586
if (AIO_WritePool_closeFile(ress.writeCtx)) {
25722587
DISPLAYLEVEL(1, "zstd: %s: %s \n", dstFileName, strerror(errno));
25732588
result = 1;
25742589
}
25752590

25762591
if (transferStat) {
2577-
UTIL_setFileStat(dstFileName, srcFileStat);
2592+
UTIL_utime(dstFileName, srcFileStat);
25782593
}
25792594

25802595
if ( (result != 0) /* operation failure */

‎programs/util.c

Lines changed: 76 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@ UTIL_STATIC void* UTIL_realloc(void *ptr, size_t size)
102102
#define chmod _chmod
103103
#endif
104104

105+
#ifndef ZSTD_HAVE_FCHMOD
106+
#if PLATFORM_POSIX_VERSION >= 199309L
107+
#define ZSTD_HAVE_FCHMOD
108+
#endif
109+
#endif
110+
111+
#ifndef ZSTD_HAVE_FCHOWN
112+
#if PLATFORM_POSIX_VERSION >= 200809L
113+
#define ZSTD_HAVE_FCHOWN
114+
#endif
115+
#endif
105116

106117
/*-****************************************
107118
* Console log
@@ -147,21 +158,38 @@ void UTIL_traceFileStat(void)
147158
g_traceFileStat = 1;
148159
}
149160

150-
int UTIL_stat(const char* filename, stat_t* statbuf)
161+
int UTIL_fstat(const int fd, const char* filename, stat_t* statbuf)
151162
{
152163
int ret;
153-
UTIL_TRACE_CALL("UTIL_stat(%s)", filename);
164+
UTIL_TRACE_CALL("UTIL_stat(%d, %s)", fd, filename);
154165
#if defined(_MSC_VER)
155-
ret = !_stat64(filename, statbuf);
166+
if (fd >= 0) {
167+
ret = !_fstat64(fd, statbuf);
168+
} else {
169+
ret = !_stat64(filename, statbuf);
170+
}
156171
#elif defined(__MINGW32__) && defined (__MSVCRT__)
157-
ret = !_stati64(filename, statbuf);
172+
if (fd >= 0) {
173+
ret = !_fstati64(fd, statbuf);
174+
} else {
175+
ret = !_stati64(filename, statbuf);
176+
}
158177
#else
159-
ret = !stat(filename, statbuf);
178+
if (fd >= 0) {
179+
ret = !fstat(fd, statbuf);
180+
} else {
181+
ret = !stat(filename, statbuf);
182+
}
160183
#endif
161184
UTIL_TRACE_RET(ret);
162185
return ret;
163186
}
164187

188+
int UTIL_stat(const char* filename, stat_t* statbuf)
189+
{
190+
return UTIL_fstat(-1, filename, statbuf);
191+
}
192+
165193
int UTIL_isRegularFile(const char* infilename)
166194
{
167195
stat_t statbuf;
@@ -183,11 +211,16 @@ int UTIL_isRegularFileStat(const stat_t* statbuf)
183211

184212
/* like chmod, but avoid changing permission of /dev/null */
185213
int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions)
214+
{
215+
return UTIL_fchmod(-1, filename, statbuf, permissions);
216+
}
217+
218+
int UTIL_fchmod(const int fd, char const* filename, const stat_t* statbuf, mode_t permissions)
186219
{
187220
stat_t localStatBuf;
188221
UTIL_TRACE_CALL("UTIL_chmod(%s, %#4o)", filename, (unsigned)permissions);
189222
if (statbuf == NULL) {
190-
if (!UTIL_stat(filename, &localStatBuf)) {
223+
if (!UTIL_fstat(fd, filename, &localStatBuf)) {
191224
UTIL_TRACE_RET(0);
192225
return 0;
193226
}
@@ -197,9 +230,20 @@ int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions)
197230
UTIL_TRACE_RET(0);
198231
return 0; /* pretend success, but don't change anything */
199232
}
200-
UTIL_TRACE_CALL("chmod");
233+
#ifdef ZSTD_HAVE_FCHMOD
234+
if (fd >= 0) {
235+
int ret;
236+
UTIL_TRACE_CALL("fchmod");
237+
ret = fchmod(fd, permissions);
238+
UTIL_TRACE_RET(ret);
239+
UTIL_TRACE_RET(ret);
240+
return ret;
241+
} else
242+
#endif
201243
{
202-
int const ret = chmod(filename, permissions);
244+
int ret;
245+
UTIL_TRACE_CALL("chmod");
246+
ret = chmod(filename, permissions);
203247
UTIL_TRACE_RET(ret);
204248
UTIL_TRACE_RET(ret);
205249
return ret;
@@ -236,19 +280,21 @@ int UTIL_utime(const char* filename, const stat_t *statbuf)
236280
}
237281

238282
int UTIL_setFileStat(const char *filename, const stat_t *statbuf)
283+
{
284+
return UTIL_setFDStat(-1, filename, statbuf);
285+
}
286+
287+
int UTIL_setFDStat(const int fd, const char *filename, const stat_t *statbuf)
239288
{
240289
int res = 0;
241290
stat_t curStatBuf;
242-
UTIL_TRACE_CALL("UTIL_setFileStat(%s)", filename);
291+
UTIL_TRACE_CALL("UTIL_setFileStat(%d, %s)", fd, filename);
243292

244-
if (!UTIL_stat(filename, &curStatBuf) || !UTIL_isRegularFileStat(&curStatBuf)) {
293+
if (!UTIL_fstat(fd, filename, &curStatBuf) || !UTIL_isRegularFileStat(&curStatBuf)) {
245294
UTIL_TRACE_RET(-1);
246295
return -1;
247296
}
248297

249-
/* set access and modification times */
250-
res += UTIL_utime(filename, statbuf);
251-
252298
/* Mimic gzip's behavior:
253299
*
254300
* "Change the group first, then the permissions, then the owner.
@@ -258,13 +304,27 @@ int UTIL_setFileStat(const char *filename, const stat_t *statbuf)
258304
* setgid bits." */
259305

260306
#if !defined(_WIN32)
261-
res += chown(filename, -1, statbuf->st_gid); /* Apply group ownership */
307+
#ifdef ZSTD_HAVE_FCHOWN
308+
if (fd >= 0) {
309+
res += fchown(fd, -1, statbuf->st_gid); /* Apply group ownership */
310+
} else
311+
#endif
312+
{
313+
res += chown(filename, -1, statbuf->st_gid); /* Apply group ownership */
314+
}
262315
#endif
263316

264-
res += UTIL_chmod(filename, &curStatBuf, statbuf->st_mode & 0777); /* Copy file permissions */
317+
res += UTIL_fchmod(fd, filename, &curStatBuf, statbuf->st_mode & 0777); /* Copy file permissions */
265318

266319
#if !defined(_WIN32)
267-
res += chown(filename, statbuf->st_uid, -1); /* Apply user ownership */
320+
#ifdef ZSTD_HAVE_FCHOWN
321+
if (fd >= 0) {
322+
res += fchown(fd, statbuf->st_uid, -1); /* Apply user ownership */
323+
} else
324+
#endif
325+
{
326+
res += chown(filename, statbuf->st_uid, -1); /* Apply user ownership */
327+
}
268328
#endif
269329

270330
errno = 0;

‎programs/util.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,15 +126,25 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const
126126
/**
127127
* Calls platform's equivalent of stat() on filename and writes info to statbuf.
128128
* Returns success (1) or failure (0).
129+
*
130+
* UTIL_fstat() is like UTIL_stat() but takes an optional fd that refers to the
131+
* file in question. It turns out that this can be meaningfully faster. If fd is
132+
* -1, behaves just like UTIL_stat() (i.e., falls back to using the filename).
129133
*/
130134
int UTIL_stat(const char* filename, stat_t* statbuf);
135+
int UTIL_fstat(const int fd, const char* filename, stat_t* statbuf);
131136

132137
/**
133138
* Instead of getting a file's stats, this updates them with the info in the
134139
* provided stat_t. Currently sets owner, group, atime, and mtime. Will only
135140
* update this info for regular files.
141+
*
142+
* UTIL_setFDStat() also takes an fd, and will preferentially use that to
143+
* indicate which file to modify, If fd is -1, it will fall back to using the
144+
* filename.
136145
*/
137146
int UTIL_setFileStat(const char* filename, const stat_t* statbuf);
147+
int UTIL_setFDStat(const int fd, const char* filename, const stat_t* statbuf);
138148

139149
/**
140150
* Set atime to now and mtime to the st_mtim in statbuf.
@@ -159,8 +169,11 @@ U64 UTIL_getFileSizeStat(const stat_t* statbuf);
159169
* Like chmod(), but only modifies regular files. Provided statbuf may be NULL,
160170
* in which case this function will stat() the file internally, in order to
161171
* check whether it should be modified.
172+
*
173+
* If fd is -1, fd is ignored and the filename is used.
162174
*/
163175
int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions);
176+
int UTIL_fchmod(const int fd, char const* filename, const stat_t* statbuf, mode_t permissions);
164177

165178
/*
166179
* In the absence of a pre-existing stat result on the file in question, these

‎tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.exact

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,40 @@ Trace:FileStat: < 0
33
Trace:FileStat: > UTIL_isConsole(2)
44
Trace:FileStat: < 0
55
Trace:FileStat: > UTIL_getFileSize(file)
6-
Trace:FileStat: > UTIL_stat(file)
6+
Trace:FileStat: > UTIL_stat(-1, file)
77
Trace:FileStat: < 1
88
Trace:FileStat: < 65537
9-
Trace:FileStat: > UTIL_stat(file)
9+
Trace:FileStat: > UTIL_stat(-1, file)
1010
Trace:FileStat: < 1
1111
Trace:FileStat: > UTIL_isDirectoryStat()
1212
Trace:FileStat: < 0
13-
Trace:FileStat: > UTIL_stat(file)
13+
Trace:FileStat: > UTIL_stat(-1, file)
1414
Trace:FileStat: < 1
1515
Trace:FileStat: > UTIL_isSameFile(file, file.zst)
16-
Trace:FileStat: > UTIL_stat(file)
16+
Trace:FileStat: > UTIL_stat(-1, file)
1717
Trace:FileStat: < 1
18-
Trace:FileStat: > UTIL_stat(file.zst)
18+
Trace:FileStat: > UTIL_stat(-1, file.zst)
1919
Trace:FileStat: < 0
2020
Trace:FileStat: < 0
2121
Trace:FileStat: > UTIL_isRegularFile(file.zst)
22-
Trace:FileStat: > UTIL_stat(file.zst)
22+
Trace:FileStat: > UTIL_stat(-1, file.zst)
2323
Trace:FileStat: < 0
2424
Trace:FileStat: < 0
2525
Trace:FileStat: > UTIL_isRegularFile(file.zst)
26-
Trace:FileStat: > UTIL_stat(file.zst)
26+
Trace:FileStat: > UTIL_stat(-1, file.zst)
2727
Trace:FileStat: < 1
2828
Trace:FileStat: < 1
2929
Trace:FileStat: > UTIL_getFileSize(file)
30-
Trace:FileStat: > UTIL_stat(file)
30+
Trace:FileStat: > UTIL_stat(-1, file)
3131
Trace:FileStat: < 1
3232
Trace:FileStat: < 65537
33-
Trace:FileStat: > UTIL_setFileStat(file.zst)
34-
Trace:FileStat: > UTIL_stat(file.zst)
33+
Trace:FileStat: > UTIL_setFileStat(4, file.zst)
34+
Trace:FileStat: > UTIL_stat(4, file.zst)
3535
Trace:FileStat: < 1
36-
Trace:FileStat: > UTIL_utime(file.zst)
37-
Trace:FileStat: < 0
3836
Trace:FileStat: > UTIL_chmod(file.zst, 0642)
39-
Trace:FileStat: > chmod
37+
Trace:FileStat: > fchmod
4038
Trace:FileStat: < 0
4139
Trace:FileStat: < 0
4240
Trace:FileStat: < 0
41+
Trace:FileStat: > UTIL_utime(file.zst)
42+
Trace:FileStat: < 0

‎tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,20 @@ Trace:FileStat: < 0
55
Trace:FileStat: > UTIL_isConsole(2)
66
Trace:FileStat: < 0
77
Trace:FileStat: > UTIL_getFileSize(file)
8-
Trace:FileStat: > UTIL_stat(file)
8+
Trace:FileStat: > UTIL_stat(-1, file)
99
Trace:FileStat: < 1
1010
Trace:FileStat: < 65537
11-
Trace:FileStat: > UTIL_stat(file)
11+
Trace:FileStat: > UTIL_stat(-1, file)
1212
Trace:FileStat: < 1
1313
Trace:FileStat: > UTIL_isDirectoryStat()
1414
Trace:FileStat: < 0
15-
Trace:FileStat: > UTIL_stat(file)
15+
Trace:FileStat: > UTIL_stat(-1, file)
1616
Trace:FileStat: < 1
1717
Trace:FileStat: > UTIL_isRegularFile(/*stdout*\)
18-
Trace:FileStat: > UTIL_stat(/*stdout*\)
18+
Trace:FileStat: > UTIL_stat(-1, /*stdout*\)
1919
Trace:FileStat: < 0
2020
Trace:FileStat: < 0
2121
Trace:FileStat: > UTIL_getFileSize(file)
22-
Trace:FileStat: > UTIL_stat(file)
22+
Trace:FileStat: > UTIL_stat(-1, file)
2323
Trace:FileStat: < 1
2424
Trace:FileStat: < 65537

‎tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.exact

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,22 @@ Trace:FileStat: < 0
33
Trace:FileStat: > UTIL_isConsole(2)
44
Trace:FileStat: < 0
55
Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
6-
Trace:FileStat: > UTIL_stat(/*stdin*\)
6+
Trace:FileStat: > UTIL_stat(-1, /*stdin*\)
77
Trace:FileStat: < 0
88
Trace:FileStat: < -1
99
Trace:FileStat: > UTIL_isSameFile(/*stdin*\, file.zst)
10-
Trace:FileStat: > UTIL_stat(/*stdin*\)
10+
Trace:FileStat: > UTIL_stat(-1, /*stdin*\)
1111
Trace:FileStat: < 0
1212
Trace:FileStat: < 0
1313
Trace:FileStat: > UTIL_isRegularFile(file.zst)
14-
Trace:FileStat: > UTIL_stat(file.zst)
14+
Trace:FileStat: > UTIL_stat(-1, file.zst)
1515
Trace:FileStat: < 0
1616
Trace:FileStat: < 0
1717
Trace:FileStat: > UTIL_isRegularFile(file.zst)
18-
Trace:FileStat: > UTIL_stat(file.zst)
18+
Trace:FileStat: > UTIL_stat(-1, file.zst)
1919
Trace:FileStat: < 1
2020
Trace:FileStat: < 1
2121
Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
22-
Trace:FileStat: > UTIL_stat(/*stdin*\)
22+
Trace:FileStat: > UTIL_stat(-1, /*stdin*\)
2323
Trace:FileStat: < 0
2424
Trace:FileStat: < -1

‎tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ Trace:FileStat: < 0
55
Trace:FileStat: > UTIL_isConsole(2)
66
Trace:FileStat: < 0
77
Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
8-
Trace:FileStat: > UTIL_stat(/*stdin*\)
8+
Trace:FileStat: > UTIL_stat(-1, /*stdin*\)
99
Trace:FileStat: < 0
1010
Trace:FileStat: < -1
1111
Trace:FileStat: > UTIL_isRegularFile(/*stdout*\)
12-
Trace:FileStat: > UTIL_stat(/*stdout*\)
12+
Trace:FileStat: > UTIL_stat(-1, /*stdout*\)
1313
Trace:FileStat: < 0
1414
Trace:FileStat: < 0
1515
Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
16-
Trace:FileStat: > UTIL_stat(/*stdin*\)
16+
Trace:FileStat: > UTIL_stat(-1, /*stdin*\)
1717
Trace:FileStat: < 0
1818
Trace:FileStat: < -1

0 commit comments

Comments
 (0)