Skip to content
This repository was archived by the owner on Oct 24, 2025. It is now read-only.

Conversation

@mgreter
Copy link
Contributor

@mgreter mgreter commented Dec 31, 2014

This is a partial fix for #88
Releated libsass PR: sass/libsass#794

mgreter added a commit that referenced this pull request Dec 31, 2014
Fix issue with input path being overwritten
@mgreter mgreter merged commit fbdc12b into sass:master Dec 31, 2014
Copy link
Contributor

Choose a reason for hiding this comment

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

@mgreter, (since I don't have definite proof with steps of reproduction, so I don't want it to create an issue for it): Sometimes, I get a debug info printed on the screen where we are setting sass_option_set_option_path, sometimes it happens on setting the next option. So do we need to guard it with condition: if(output) like here? Don't we have these conditional guards on libsass side, as we pass nullptr for every empty option string we receive to libsass?

PS: Sometimes == extremely occasional (probably when I am doing some random testing and debugging some weird scenarios, I am unable to draw definite pattern for it till now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably taken over from previous code!
It should be protected in libsass itself, but the code line above actually has some different meaning, since it would overwrite the previous output path in that case. You also did not include which debug info gets printed!?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I will capture it next time. It was invalid instruction or SIGILL (and the first line mentioned glibc). I will try to capture it next time.

Thanks for the clarification on settings! 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants