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

Ensure that fileno() returns -1 when given an invalid file pointer. #2157

Merged
merged 1 commit into from
Feb 26, 2014
Merged

Ensure that fileno() returns -1 when given an invalid file pointer. #2157

merged 1 commit into from
Feb 26, 2014

Conversation

rfk
Copy link
Contributor

@rfk rfk commented Feb 25, 2014

Previously this would throw since getStreamFromPtr returns undefined for invalid file pointers.

@@ -2420,7 +2420,9 @@ LibraryManager.library = {
fileno: function(stream) {
// int fileno(FILE *stream);
// http://pubs.opengroup.org/onlinepubs/000095399/functions/fileno.html
return FS.getStreamFromPtr(stream).fd;
stream = FS.getStreamFromPtr(stream);
if (!stream) return -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

As part of #20, could you break the conditional into three lines:

if (!stream) {
  return -1;
}

@rfk
Copy link
Contributor Author

rfk commented Feb 25, 2014

rebased with style tweak

@haneefmubarak
Copy link
Contributor

thx @rfk!

@kripken
Copy link
Member

kripken commented Feb 25, 2014

Sorry to hassle you here, but actually that was not accurate - it is valid to do if (!stream) return -1 on the same line, in fact it is recommended in our conventions.

@haneefmubarak , this might not be a common convention in other projects, but it is used throughout emscripten.

What is not acceptable in emscripten is

if (x)
  y

, we disallow that without curly braces. But, braces are not needed for short stuff fitting on the same line.

@kripken
Copy link
Member

kripken commented Feb 25, 2014

With that style thing reverted, this looks good. @rfk , any chance you can make a small testcase for this or add to an existing one, so we don't regress it later?

@rfk
Copy link
Contributor Author

rfk commented Feb 26, 2014

rebased and added stand-alone testcase for fileno() with both valid and invalid stream pointer.

kripken added a commit that referenced this pull request Feb 26, 2014
Ensure that fileno() returns -1 when given an invalid file pointer.
@kripken kripken merged commit 4c015a3 into emscripten-core:incoming Feb 26, 2014
@kripken
Copy link
Member

kripken commented Feb 26, 2014

Thanks!

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.

3 participants