Skip to content

Commit

Permalink
src: remove uid_t/gid_t casts
Browse files Browse the repository at this point in the history
If uid_t/gid_t are uint32_t, then the casts are unnecessary. This
appears to be true in all recent versions of all supported platforms,
so this change makes that assumption explicit and removes the casts.

Conversely, if uid_t/gid_t are smaller unsigned integer types (such as
uint16_t in earlier versions of Linux) or signed integer types (such as
int32_t), then the casts are potentially dangerous because they might
change the value of the uid/gid. If this happens on any platform, the
added static_assert will fail, and additional bound checks should be
introduced.

PR-URL: #44914
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
tniessen authored and danielleadams committed Oct 11, 2022
1 parent 8c69da8 commit 9f2dd48
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/node_credentials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ static const char* name_by_gid(gid_t gid) {

static uid_t uid_by_name(Isolate* isolate, Local<Value> value) {
if (value->IsUint32()) {
return static_cast<uid_t>(value.As<Uint32>()->Value());
static_assert(std::is_same<uid_t, uint32_t>::value);
return value.As<Uint32>()->Value();
} else {
Utf8Value name(isolate, value);
return uid_by_name(*name);
Expand All @@ -224,7 +225,8 @@ static uid_t uid_by_name(Isolate* isolate, Local<Value> value) {

static gid_t gid_by_name(Isolate* isolate, Local<Value> value) {
if (value->IsUint32()) {
return static_cast<gid_t>(value.As<Uint32>()->Value());
static_assert(std::is_same<gid_t, uint32_t>::value);
return value.As<Uint32>()->Value();
} else {
Utf8Value name(isolate, value);
return gid_by_name(*name);
Expand Down

0 comments on commit 9f2dd48

Please sign in to comment.