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

Use isnan(3) to check for NaN #1

Merged
merged 2 commits into from
Apr 18, 2014
Merged

Use isnan(3) to check for NaN #1

merged 2 commits into from
Apr 18, 2014

Conversation

swolchok
Copy link
Contributor

There's a standard C function (actually a macro) to check for NaN; let's use that for clarity.

@swolchok
Copy link
Contributor Author

Also pushed 5498516 which fixes a memory leak

@swolchok
Copy link
Contributor Author

btw to find the bug fixed by 5498516:
brew install valgrind
clang Layout.c
valgrind ./a.out
note complaint about "definitely lost" bytes, follow advice to re-run with --leak-check=full:
valgrind --leak-check=full ./a.out

vjeux added a commit that referenced this pull request Apr 18, 2014
Use isnan(3) to check for NaN
@vjeux vjeux merged commit 1d60193 into facebook:master Apr 18, 2014
@vjeux
Copy link
Contributor

vjeux commented Apr 18, 2014

Thanks! I definitively need to install valgrind :)

@mrjjwright mrjjwright mentioned this pull request Mar 8, 2015
aaronechiu added a commit that referenced this pull request Sep 16, 2015
Make CSSLayout constants public
facebook-github-bot pushed a commit that referenced this pull request Sep 25, 2019
Summary:
In D17439957, I noted that YogaLogger#log throws a NoMethodFoundException when called from C++ b/c C++ and Java's signatures of that method don't match. C++ uses YogaNodeJNIBase for the first param, Java uses YogaNode. Both my attempts to fix this failed.

Attempt #1 - Make Java use YogaNodeJNIBase. This doesn't work because the :java-interface target includes YogaLogger but not YogaNodeJNIBase. Moving YogaLogger to the impl target doesn't work either b/c other files in :java-interface reference YogaLogger.

Attempt #2 - Make C++ use YogaNode. This doesn't work b/c we try to call the log method with objects of fbjni type YogaNodeJNIBase. This would be fine in Java since YogaNodeJNIBase extends YogaNode. But fbjni's typing isn't advanced enough to know this, so the Yoga C++ fails to compile.

At this point, I was wondering what the value of having this param in the log function at all was. None of the implementations in our codebase use it today. It might be easier to just remove it all together. This also removes a bug with YGNodePrint where we pass a null layout context that eventually causes a SIG_ABRT when we use it to try to find a YogaNode to pass to this function. (https://fburl.com/diffusion/ssw9h8lv).

Reviewed By: amir-shalem

Differential Revision: D17470379

fbshipit-source-id: 8fc2d95505971a52af2399a9fbb60b63f27f0ec2
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.

2 participants