Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: fix stat dev unsigned cast overflow #16705

Closed
wants to merge 2 commits into from
Closed

fs: fix stat dev unsigned cast overflow #16705

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c623ad0
fs: fix stat dev unsigned cast overflow
JLHwung Nov 3, 2017
4a88989
fs: use Number::New since all fields are uint64_t
JLHwung Nov 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 7 additions & 28 deletions src/node_file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,47 +438,26 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
// We need to check the return value of Number::New() and Date::New()
// and make sure that we bail out when V8 returns an empty handle.

// Unsigned integers. It does not actually seem to be specified whether
// uid and gid are unsigned or not, but in practice they are unsigned,
// and Node’s (F)Chown functions do check their arguments for unsignedness.
// Numbers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment looks pretty out of place now. Care to just remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments are added on e9ce8fc to accompany different X macro definition. I don't think it is out of place and we are good to let it be.

#define X(name) \
Local<Value> name = Integer::NewFromUnsigned(env->isolate(), s->st_##name); \
Local<Value> name = Number::New(env->isolate(), \
static_cast<double>(s->st_##name)); \
Copy link
Member

@bnoordhuis bnoordhuis Nov 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed the name.IsEmpty() check but it should most definitely stay.

(edit: looking at the wrong diff.)

if (name.IsEmpty()) \
return Local<Object>(); \

X(uid)
X(gid)
# if defined(__POSIX__)
X(blksize)
# else
Local<Value> blksize = Undefined(env->isolate());
# endif
#undef X

// Integers.
#define X(name) \
Local<Value> name = Integer::New(env->isolate(), s->st_##name); \
if (name.IsEmpty()) \
return Local<Object>(); \

X(ino)
X(size)
X(dev)
X(mode)
X(nlink)
X(rdev)
#undef X

// Numbers.
#define X(name) \
Local<Value> name = Number::New(env->isolate(), \
static_cast<double>(s->st_##name)); \
if (name.IsEmpty()) \
return Local<Object>(); \

X(ino)
X(size)
# if defined(__POSIX__)
X(blksize)
X(blocks)
# else
Local<Value> blksize = Undefined(env->isolate());
Local<Value> blocks = Undefined(env->isolate());
# endif
#undef X
Expand Down