Skip to content

Commit 01c7de7

Browse files
committed
src: example use of gsl::narrow to uncover a bug
1 parent 8b05c27 commit 01c7de7

File tree

5 files changed

+120
-15
lines changed

5 files changed

+120
-15
lines changed

deps/GSL/gsl/gsl_assert

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,15 @@ namespace details
125125
#if defined(GSL_TERMINATE_ON_CONTRACT_VIOLATION)
126126

127127
template <typename Exception>
128-
[[noreturn]] void throw_exception(Exception&&) noexcept
128+
[[noreturn]] void throw_exception(Exception&& ex) noexcept
129129
{
130-
gsl::details::terminate();
130+
static const char* const args[] = {
131+
__FILE__,
132+
STRINGIFY(__LINE__),
133+
ex.what(),
134+
PRETTY_FUNCTION_NAME
135+
};
136+
node::Assert(&args);
131137
}
132138

133139
#else

doc/api/fs.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,8 @@ The numeric identifier of the device containing the file.
638638

639639
The file system specific "Inode" number for the file.
640640

641+
*Note*: The `number` version is unreliable on Windows as values often overflow.
642+
641643
### stats.mode
642644

643645
* {number|bigint}

node.gypi

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
'deps/v8/include',
3030
'src',
3131
],
32+
'defines': [ 'GSL_TERMINATE_ON_CONTRACT_VIOLATION', ],
3233
# Putting these explicitly here so not to be dependant on common.gypi.
3334
'cflags': [ '-Wall', '-Wextra', '-Wno-unused-parameter', ],
3435
'xcode_settings': {

src/node_file.h

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "node.h"
77
#include "stream_base.h"
88
#include "req_wrap-inl.h"
9+
#include <gsl/gsl_util>
910

1011
namespace node {
1112

@@ -193,24 +194,25 @@ constexpr uint64_t ToNative(uv_timespec_t ts) {
193194
template <typename NativeT, typename V8T>
194195
constexpr void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields,
195196
const uv_stat_t* s, const size_t offset = 0) {
196-
fields->SetValue(offset + 0, s->st_dev);
197-
fields->SetValue(offset + 1, s->st_mode);
198-
fields->SetValue(offset + 2, s->st_nlink);
199-
fields->SetValue(offset + 3, s->st_uid);
200-
fields->SetValue(offset + 4, s->st_gid);
201-
fields->SetValue(offset + 5, s->st_rdev);
197+
fields->SetValue(offset + 0, gsl::narrow<NativeT>(s->st_dev));
198+
fields->SetValue(offset + 1, gsl::narrow<NativeT>(s->st_mode));
199+
fields->SetValue(offset + 2, gsl::narrow<NativeT>(s->st_nlink));
200+
fields->SetValue(offset + 3, gsl::narrow<NativeT>(s->st_uid));
201+
fields->SetValue(offset + 4, gsl::narrow<NativeT>(s->st_gid));
202+
fields->SetValue(offset + 5, gsl::narrow<NativeT>(s->st_rdev));
202203
#if defined(__POSIX__)
203-
fields->SetValue(offset + 6, s->st_blksize);
204+
fields->SetValue(offset + 6, gsl::narrow<NativeT>(s->st_blksize));
205+
fields->SetValue(offset + 7, gsl::narrow<NativeT>(s->st_ino));
206+
fields->SetValue(offset + 8, gsl::narrow<NativeT>(s->st_size));
207+
fields->SetValue(offset + 9, gsl::narrow<NativeT>(s->st_blocks));
204208
#else
205209
fields->SetValue(offset + 6, 0);
206-
#endif
207-
fields->SetValue(offset + 7, s->st_ino);
208-
fields->SetValue(offset + 8, s->st_size);
209-
#if defined(__POSIX__)
210-
fields->SetValue(offset + 9, s->st_blocks);
211-
#else
210+
// This overflows on Windows for NativeT == double
211+
fields->SetValue(offset + 7, gsl::narrow_cast<NativeT>(s->st_ino));
212+
fields->SetValue(offset + 8, gsl::narrow<NativeT>(s->st_size));
212213
fields->SetValue(offset + 9, 0);
213214
#endif
215+
214216
// Dates.
215217
fields->SetValue(offset + 10, ToNative<NativeT>(s->st_atim));
216218
fields->SetValue(offset + 11, ToNative<NativeT>(s->st_mtim));
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
6+
if (!common.isWindows)
7+
assert.fail('Code should fail only on Windows.');
8+
9+
const fs = require('fs');
10+
const promiseFs = require('fs').promises;
11+
const path = require('path');
12+
const tmpdir = require('../common/tmpdir');
13+
const { isDate } = require('util').types;
14+
15+
tmpdir.refresh();
16+
17+
let testIndex = 0;
18+
19+
function getFilename() {
20+
const filename = path.join(tmpdir.path, `test-file-${++testIndex}`);
21+
fs.writeFileSync(filename, 'test');
22+
return filename;
23+
}
24+
25+
function verifyStats(bigintStats, numStats) {
26+
assert.ok(Number.isSafeInteger(numStats.ino));
27+
assert.strictEqual(bigintStats.ino, BigInt(numStats.ino));
28+
}
29+
30+
{
31+
const filename = getFilename();
32+
const bigintStats = fs.statSync(filename, { bigint: true });
33+
const numStats = fs.statSync(filename);
34+
verifyStats(bigintStats, numStats);
35+
}
36+
37+
{
38+
const filename = __filename;
39+
const bigintStats = fs.statSync(filename, { bigint: true });
40+
const numStats = fs.statSync(filename);
41+
verifyStats(bigintStats, numStats);
42+
}
43+
44+
{
45+
const filename = __dirname;
46+
const bigintStats = fs.statSync(filename, { bigint: true });
47+
const numStats = fs.statSync(filename);
48+
verifyStats(bigintStats, numStats);
49+
}
50+
51+
{
52+
const filename = getFilename();
53+
const fd = fs.openSync(filename, 'r');
54+
const bigintStats = fs.fstatSync(fd, { bigint: true });
55+
const numStats = fs.fstatSync(fd);
56+
verifyStats(bigintStats, numStats);
57+
fs.closeSync(fd);
58+
}
59+
60+
{
61+
const filename = getFilename();
62+
fs.stat(filename, { bigint: true }, (err, bigintStats) => {
63+
fs.stat(filename, (err, numStats) => {
64+
verifyStats(bigintStats, numStats);
65+
});
66+
});
67+
}
68+
69+
{
70+
const filename = getFilename();
71+
const fd = fs.openSync(filename, 'r');
72+
fs.fstat(fd, { bigint: true }, (err, bigintStats) => {
73+
fs.fstat(fd, (err, numStats) => {
74+
verifyStats(bigintStats, numStats);
75+
fs.closeSync(fd);
76+
});
77+
});
78+
}
79+
80+
(async function() {
81+
const filename = getFilename();
82+
const bigintStats = await promiseFs.stat(filename, { bigint: true });
83+
const numStats = await promiseFs.stat(filename);
84+
verifyStats(bigintStats, numStats);
85+
})();
86+
87+
(async function() {
88+
const filename = getFilename();
89+
const handle = await promiseFs.open(filename, 'r');
90+
const bigintStats = await handle.stat({ bigint: true });
91+
const numStats = await handle.stat();
92+
verifyStats(bigintStats, numStats);
93+
await handle.close();
94+
})();

0 commit comments

Comments
 (0)