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.

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: properly handle fd passed to truncate() #866

Closed
wants to merge 2 commits into from
Closed

fs: properly handle fd passed to truncate() #866

wants to merge 2 commits into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Feb 17, 2015

Currently, fs.truncate() silently fails when a file descriptor is passed as the first argument. This commit changes this behavior to properly call fs.ftruncate(). This commit also adds proper type checking to the callback provided to makeCallback().

Port of nodejs/node-v0.x-archive#9161

fs.truncate(fd, 5, function(err) {
assert.ok(!err);
success++;
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: check that the file is really truncated.

@bnoordhuis
Copy link
Member

LGTM with the remark that, ideally, the commit is split in two. It fixes two unrelated bugs now.

bjouhier and others added 2 commits February 18, 2015 12:53
Currently, fs.truncate() silently fails when a file descriptor
is passed as the first argument. This commit changes this
behavior to properly call fs.ftruncate().

PR-URL: nodejs/node-v0.x-archive#9161
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Conflicts:
	lib/fs.js
This commit adds proper type checking to makeCallback(). Anything
other than undefined or a function will throw.

PR-URL: #866
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@cjihrig
Copy link
Contributor Author

cjihrig commented Feb 18, 2015

@bnoordhuis added a check that the truncate() worked, split into two commits, and added a test for the makeCallback() change. PTAL

return rethrow();
}

if (typeof cb !== 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of an extra check? this function is internal anyway never mind

@vkurchatkin
Copy link
Contributor

LGTM

cjihrig added a commit that referenced this pull request Feb 21, 2015
This commit adds proper type checking to makeCallback(). Anything
other than undefined or a function will throw.

PR-URL: #866
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Vladimir Kurchatkin <vladimir.kurchatkin@gmail.com>
@cjihrig
Copy link
Contributor Author

cjihrig commented Feb 21, 2015

Thanks! Landed in c82e580 and 1f40b2a

@cjihrig cjihrig closed this Feb 21, 2015
@cjihrig cjihrig deleted the truncate branch February 21, 2015 17:26
This was referenced Feb 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants