Skip to content

Commit

Permalink
fs: cleanup fd lchown and lchownSync
Browse files Browse the repository at this point in the history
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?

PR-URL: #18329
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
jasnell authored and evanlucas committed Jan 30, 2018
1 parent 6ae7bb1 commit 230a102
Showing 1 changed file with 19 additions and 11 deletions.
30 changes: 19 additions & 11 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1110,20 +1110,16 @@ if (constants.O_SYMLINK !== undefined) {
};

fs.lchmodSync = function(path, mode) {
var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);

// Prefer to return the chmod error, if one occurs,
// but still try to close, and report closing errors if they occur.
var ret;
let ret;
try {
ret = fs.fchmodSync(fd, mode);
} catch (err) {
try {
fs.closeSync(fd);
} catch (ignore) {}
throw err;
} finally {
fs.closeSync(fd);
}
fs.closeSync(fd);
return ret;
};
}
Expand Down Expand Up @@ -1155,13 +1151,25 @@ if (constants.O_SYMLINK !== undefined) {
callback(err);
return;
}
fs.fchown(fd, uid, gid, callback);
// Prefer to return the chown error, if one occurs,
// but still try to close, and report closing errors if they occur.
fs.fchown(fd, uid, gid, function(err) {
fs.close(fd, function(err2) {
callback(err || err2);
});
});
});
};

fs.lchownSync = function(path, uid, gid) {
var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
return fs.fchownSync(fd, uid, gid);
const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
let ret;
try {
ret = fs.fchownSync(fd, uid, gid);
} finally {
fs.closeSync(fd);
}
return ret;
};
}

Expand Down

0 comments on commit 230a102

Please sign in to comment.