Assertion error when fs.futimes() is called and futime() is not supported.

[shigeki]

As described the same case of the pull request of #2050, test/simple/test-fs-utimes.js was failed with assertion error at assert(r == 0); of src/node_file.cc:279 when futime(3) is not supported on the platform because uv_fs_futime() returns -1.

There seems to have several options to fix.

option #0: Let this as is, fs.futimesSync() throw ENOSYS error and fs.futimes() get an assertion error.
option #1: Both throws ENOSYS error
option #2: fs.futimes() have a callback with ENOSYS error.

I made a patch of option #2 as below but I'm not sure that this also works fine at ENOMEM error cases

shigeki/node@8663b64

I would like to know if there has another alternatives to fix this issue.

[bnoordhuis]

Is shigeki/node@8663b64 still relevant?

[shigeki]

Ben, I've found that common.gypi has NDEBUG flag in Release mode so that all assert(3) are ignored, which surprised me a lot.
That means that this issue was solved because no assertion errors raised in Release build at all, however, with my patch fs.futimes returns proper error in case of no futime() as shown in below. So, I think it still relevant.

var fs = require('fs');
var fd = fs.openSync(__filename, 'r');
fs.futimes(fd, new Date(), new Date(), function(err) {
   console.log(err);
});

 ./node ~/futime.js
{ [Error: ENOSYS, function not implemented] errno: 35, code: 'ENOSYS' }

[bnoordhuis]

Right you are, sir. I've landed your patch (slightly reworked to make it apply to the current master) in 2156e5e. Thanks!

[shigeki]

Thanks for merge, Ben. One comment was noted on the patch.
As for the NDEBUG flag in common.gypi, I think assert(3) is needed to work even in Release mode to check node run properly. Had I better to submit a new ticket or not?

[bnoordhuis]

I've disabled NDEBUG in 6b58537. I'll re-enable it eventually but probably not until 1.0.

[shigeki]

That's fine. Thanks.