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

Remove redundant assert #4918

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

kamil-tekiela
Copy link
Contributor

Signed-off-by: Kamil Tekiela tekiela246@gmail.com

Q A
Type improvement
BC Break no
Fixed issues Code cleanup

Summary

The function mysqli_init can only return false when the initialization fails. The initialization can only fail when there's not enough system memory, in which case this function returning false is the least of the problems. There's no reason to assert that this function didn't return false. If we want to avoid FP static analyser warnings then we can use the OO style (which I also think should be used to follow the coding style as I can't see any other mysqli function used in procedural style). This makes the code cleaner and easier to understand.

Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
@derrabus derrabus added this to the 3.2.0 milestone Oct 22, 2021
@morozov
Copy link
Member

morozov commented Oct 22, 2021

If we want to avoid FP static analyser warnings [...]

Yes, that was the primary reason to introduce this assertion (#4524). Thanks for the patch!

How come that being aliases, mysqli_init() does report the failure via the return values but new mysqli() doesn't? Will the latter throw an exception in case of an initialization failure?

@morozov morozov merged commit 7aeac08 into doctrine:3.2.x Oct 22, 2021
@kamil-tekiela
Copy link
Contributor Author

How come that being aliases, mysqli_init() does report the failure via the return values but new mysqli() doesn't?

I have wondered about the same and this bugs me a little, but I don't know if it's worth doing anything about it now. The procedural style is weird. It only returns objects on success. While it's extremely unlikely that mysqli_init() could ever return false this is the accepted style. If something would ever go wrong then this function will return false instead of an object. mysqli_connect() behaves similarly, but there exists a real possibility of it actually failing (when error reporting is silenced).

I don't know what would need to happen for mysqlnd to not be able to initialize the object properly. If mysqli is compiled with libmysqlclient then it's possible that the libmysqlclient cannot allocate memory and it will return NULL. I think this is the only possible way that mysqli_init() could return false.

Will the latter throw an exception in case of an initialization failure?

No, it will fail silently, but that should not be a huge problem. You can't use uninitialized mysqli object anyway. If you tried calling real_connect() on such object you would get an error:

mysqli object is already closed

@kamil-tekiela kamil-tekiela deleted the Remove-redundant-assert branch October 22, 2021 17:44
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants