Skip to content

Commit

Permalink
fix condition
Browse files Browse the repository at this point in the history
  • Loading branch information
@raszi
raszi committed Mar 1, 2015
1 parent 4f7df8f commit d1e8eef
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/tmp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ function _createTmpFile(options, callback) {
fs.closeSync(fdPath[0]);
} catch (e) {
// did user close the fd?
if (e.errno === -1 * _c.EBADF) {
if (e.errno !== -1 * _c.EBADF) {
// no, rethrow
throw e;
}
Expand Down

6 comments on commit d1e8eef

@silkentrance
Copy link
Collaborator

Choose a reason for hiding this comment

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

Considering the original

Math.abs(e.errno) != _c.EBADF

which was entirely correct... 😁

@raszi
Copy link
Owner Author

@raszi raszi commented on d1e8eef Mar 9, 2015

Choose a reason for hiding this comment

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

I don't agree with that. What if they will introduce a new error code, or somebody returns an error code like that which has no connection with this EBADF?

@silkentrance
Copy link
Collaborator

Choose a reason for hiding this comment

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

@raszi Now, if they will ever introduce another error code, then it will necessarily fail as it cannot foresee the future. The same applies to "for somebody returning something...", yet, I guess people who monkey patch the fs will make sure that it will behave in exactly the same way.

@silkentrance
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW: the commit is rather hilarious: DIEBEEF 😁

@silkentrance
Copy link
Collaborator

Choose a reason for hiding this comment

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

While we are at it, why not simply test for

if (-e.errno == _c.EBADF)

No need for the multiplication and also no need for testing against idempotency '==='.

@pabigot
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, based on this discussion it may be better to ignore errno and use code which always has a string value. The sign and interpretation of errno values is not consistent nor well-defined in Node.

if (e.code === 'EBADF')

Please sign in to comment.