I think Number.isFinite does both checks in one call

@targos Ah, you are correct. I updated now. PTAL.

Just seeing the error message. If you want to ensure it is an integer, you can drop Number.isFinite and use Number.isInteger instead

@targos Man, that's wonderful. I didn't know about that function and that is exactly what we need here. Thanks :-) Updated the PR.

Change in functionality. Here's an approximate test:

if (typeof path === 'number' && !Number.isInteger(path))
  throw new TypeError('Not an integer');

or are you just short-circuiting for the fast path?

This is a legacy functionality it seems. If we pass an fd to truncate it internally delegates it to ftruncate. So we cannot throw error here. We are just checking if it is a positive integer and move on to the next check if is not.

Are you suggesting to not to call ftruncate at all?

I just followed the logic paths for current and this patch, and there seemed to be a discrepancy.

@trevnorris Agree, but wouldn't this check be better?

My assumption was to land most these changes on a minor, then making any breaking changes, that push this to semver-major, to another PR. If you're cool with this staying on major then I'll have another look from that perspective.

@trevnorris I am okay with this being in major :-) Let's get this done right. Please suggest improvements as well :-)

Change this and the below line to:

CHECK(args[1]->IsNumber());
const int64_t = args[1]->IntegerValue();

Reason for the CHECK() is because we're now making an assumption about the incoming value. Always want to catch that if incorrect, because at this point if the value is incorrect then we've screwed up.

wait. nm. read this as the fd value, not the len value.

EDIT: meaning, no need for the CHECK(). can still collapse the other.

Change this to a TypeError. Also I'd replace "positive" with "unsigned".

Done!