Skip to content

error: use int64_t as reference counter #5018

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

Closed
wants to merge 1 commit into from

Conversation

dmitryikh
Copy link
Contributor

@dmitryikh dmitryikh commented May 23, 2020

Before it was unsafe to use error.prev, box.error.last(), or any error
method that increment error reference counter. Any of them can throw
a Lua error in case of INT32_MAX overflow.

This patch uses int64_t as error reference counter making it impossible
to get counter overflow.

Closes #4902

Unfortunately I don't know how to write tests on this behaviour..

@Gerold103 Gerold103 self-requested a review May 27, 2020 17:20
@dmitryikh
Copy link
Contributor Author

Should I send the PR to mailing list?

@Gerold103
Copy link
Collaborator

I will take a look here.

@Gerold103
Copy link
Collaborator

Gerold103 commented Jun 2, 2020

error: use int64_t as reference counter

Before it was unsafe to use error.prev, box.error.last(), or any error
method that increment error reference cunter. Any of them now can throw
a Lua error in case of INT32_MAX overflow.

This patch uses int64_t as error reference counter making it impossible
to get counter overflow.

Closes #4902

Typo: 'increment' -> 'increments'. Also in the commit message we describe the old behaviour using past tense. You said "any of them now can throw", but you should say "any of them could throw".

Before it was unsafe to use error.prev, box.error.last(), or any error
method that increments error reference counter. Any of them could throw a
Lua error in case of INT32_MAX overflow.

This patch uses int64_t as error reference counter making it impossible
to get counter overflow.

Closes tarantool#4902
@dmitryikh dmitryikh force-pushed the error_ref_to_int64 branch from d25d373 to c4f0862 Compare June 4, 2020 00:31
@dmitryikh
Copy link
Contributor Author

Thanks, @Gerold103, for review!

I fixed the issues.

@dmitryikh dmitryikh requested a review from Gerold103 June 5, 2020 21:16
@Gerold103
Copy link
Collaborator

You may need a second review, and someone who will merge. @kyukhin probably.

@kyukhin
Copy link
Contributor

kyukhin commented Jun 8, 2020

Could you please send your final patch to tarantool-patches mailing list?

@dmitryikh
Copy link
Contributor Author

Could you please send your final patch to tarantool-patches mailing list?

done

@Totktonada
Copy link
Member

It was pushed as 2.5.0-129-g2e3c81dee to master and as 2.4.1-63-g56b57be53 to 2.4.

@kyukhin I updated drafts of 2.5.1 and 2.4.2 release notes.

@Totktonada Totktonada closed this Jun 9, 2020
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.

Make struct error reference counter uint64/int64
4 participants