-
Notifications
You must be signed in to change notification settings - Fork 9
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
small: bump new version #60
Conversation
b778311
to
da01d0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I would include the performance degradation fix. It does not change API / ABI. All three commits upward this one should not change ABI anyhow, so I would pick up them too and just update to the latest master.
-
Please, leave a few words why
small
need to be updated. -
Re c99: As far as I get the change where the problem was introduced, the idea was to inherit parent project's CFLAGS and so libsmall should support C89. I filed the issue about this. I think it would be better to fix it and pick up (if you'll not, please, at least link it somewhere).
However it seems that, despite the problem in
small
exists, we can workaround it here in some not very intruisive way: just move CFLAGS setting above libsmall CMake files inclusion.
The main reason of update: When the memcached module is linked with tarantool, it uses some of the "small" functions from tarantool (which were exported by tarantool, for example: mempool_create_with_order()) and some from the "small" with wich it was built (for example: mempool_create(), mempool_free()). This can lead to undefined behavior (segfault for example). To avoid the problem, will use the same (close) version of the "small" as tarantool. Part of #59
da01d0d
to
b1e2d94
Compare
If SMALL_EMBEDDED is used, "small" will try to compile with the default flags, but it can't be compiled with std=c89. Look like a "small" library bug. So, we will use std=c99 globally as workaround. See tarantool/small#25 Part of #59 @Totktonada: added the comment.
b1e2d94
to
6852c5a
Compare
Added a comment for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
The "small" library version has been updated.
Part of #59