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

Makes NSError** argument __autoreleasing in scell_token, adds check a… #259

Merged
merged 5 commits into from Dec 11, 2017

Conversation

Projects
None yet
2 participants
@valeriyvan
Contributor

valeriyvan commented Dec 8, 2017

…gainst null dereference

@vixentael

This comment has been minimized.

Member

vixentael commented Dec 8, 2017

second part of #257 ?

@valeriyvan

This comment has been minimized.

Contributor

valeriyvan commented Dec 8, 2017

TL DR: same fix, but for other files.
Confused by same branch name?
All NSError ** should be __autoreleasing, otherwise it might lead to appearing of zombies.
And it's better to be ready for NSError* passed to be nil. Because it's common practice ignore detailed error info which NSError provides when method signals for error with nil return value. In that case NSError provides additional information which we don't always need.

@vixentael

This comment has been minimized.

Member

vixentael commented Dec 8, 2017

yes, you're right, we should expect nil as errors.

can you please update smessage and ssession classes too? just to be consistent everywhere

@vixentael vixentael self-assigned this Dec 8, 2017

@vixentael vixentael merged commit 221783b into cossacklabs:master Dec 11, 2017

2 checks passed

ci/bitrise/b32b4ea8bffedad7/pr Passed - themis
Details
ci/circleci Your tests passed on CircleCI!
Details
@vixentael

This comment has been minimized.

Member

vixentael commented Dec 11, 2017

Nah, our tests and examples are using themis pod from master branch, so CI can't indicate immediately that tests are failing.

I've found and fixed small issues in your PR
ad36068
b38fa55

and merged it.

Thank you a lot!

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