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

Include correct header for malloc related functions. #658

Merged
merged 1 commit into from
Feb 10, 2015

Conversation

ahacking
Copy link

The C malloc() functions require inclusion of stdlib.h

This fixes compilation on FreeBSD.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.31% when pulling 2ce6748 on ahacking:master into 9c3accb on sass:master.

@@ -1,3 +1,4 @@
#include <stdlib.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would not be necessary, if you add it once in src/sass_context_wrapper.h.

@am11
Copy link
Contributor

am11 commented Feb 10, 2015

Thanks for the contribution. 👍

I have left some comments, PTAL.

@ahacking
Copy link
Author

Moved <stdlib.h> to common header and rebased.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.59% when pulling 5f6c802 on ahacking:master into 1180417 on sass:master.

@ahacking
Copy link
Author

@am11 Including <stdio.h> is correct. Failing to include <stdlib.h> means malloc() (if not being defined fortuitously through some indirect include) will be treated as returning int which is wrong. It may still 'work' if you're lucky when sizeof(int) == sizeof(void*).

Taking the responsible approach when using malloc/calloc/free is to include <stdlib.h> as per the C standard. If we were including for example a Linux/glibc non-standard header like <malloc.h> then yes that deserves being surrounded in an #ifdefguard, but putting a platform specific guard for an expressly portable inclusion is a bad idea and just increases risk of bad things happening and also creates unnecessary maintenance.

I am finding that master of node-sass generates a Segmentation Fault with my app.scss. I have run the tests and whilst there is a timeout failure with the huge test, there isn't a segfault, so my app.scss is triggering a regression (it works fine with 1.x). I haven't been able to determine where or what in the code is causing the problem yet but if I can isolate the cause I will open a separate issue.

Here is the failing npm-debug.log:

[andrew@hacklap ~/proj/node-sass]$ less npm-debug.log 
0 info it worked if it ends with ok
1 verbose cli [ '/usr/home/andrew/.local/bin/node',
1 verbose cli   '/usr/home/andrew/.local/bin/npm',
1 verbose cli   'run-script',
1 verbose cli   'test' ]
2 info using npm@1.4.28
3 info using node@v0.10.36
4 verbose run-script [ 'pretest', 'test', 'posttest' ]
5 info pretest node-sass@2.0.0-beta
6 verbose unsafe-perm in lifecycle true
7 info test node-sass@2.0.0-beta
8 verbose unsafe-perm in lifecycle true
9 info node-sass@2.0.0-beta Failed to exec test script
10 error node-sass@2.0.0-beta test: `node_modules/.bin/mocha test`
10 error Exit status 1
11 error Failed at the node-sass@2.0.0-beta test script.
11 error This is most likely a problem with the node-sass package,
11 error not with npm itself.
11 error Tell the author that this fails on your system:
11 error     valgrind node_modules/.bin/mocha test
11 error You can get their info via:
11 error     npm owner ls node-sass
11 error There is likely additional logging output above.
12 error System FreeBSD 10.1-STABLEJAN2015
13 error command "/usr/home/andrew/.local/bin/node" "/usr/home/andrew/.local/bin/npm" "run-script" "test"
14 error cwd /usr/home/andrew/proj/node-sass
15 error node -v v0.10.36
16 error npm -v 1.4.28
17 error code ELIFECYCLE
18 verbose exit [ 1, true ]

am11 added a commit that referenced this pull request Feb 10, 2015
Include correct header for malloc related functions.
@am11 am11 merged commit fe1d59f into sass:master Feb 10, 2015
@am11
Copy link
Contributor

am11 commented Feb 10, 2015

@ahacking, thanks! 🎉

Regarding the issue, please open it in the issue tracker with steps to reproduce the error: Sass code, command you used to execute etc.

@am11
Copy link
Contributor

am11 commented Feb 10, 2015

@ahacking, note that it could be due to the unsupported language features in the .scss file. Usually segfaults are due to LibSass code (but not necessarily).

jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
Add support for the not expression
jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
This reverts commit b2f57a7, reversing
changes made to c0dbb43.
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.

None yet

3 participants