Skip to content

Fix -Werror=sign-compare warning in zend_memnistr when compiling with -Werror #8042

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

Merged
merged 1 commit into from
Feb 5, 2022

Conversation

TysonAndre
Copy link
Contributor

comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘long int’ [-Werror=sign-compare]

This is asserting end >= haystack as a precondition for callers (in debug
builds) and existing callers are correct.

An alternate option is to cast the left side to int64_t, but that may be
slightly inefficient for 32-bit builds (to account for >=2GB strings).

Noticed when locally developing with -Werror

`error: comparison of integer expressions of different signedness: ‘size_t’ {aka
‘long unsigned int’} and ‘long int’ [-Werror=sign-compare]`

This is asserting `end >= haystack` as a precondition for callers (in debug
builds) and existing callers are correct.

An alternate option is to cast the left side to `int64_t`, but that may be
slightly inefficient for 32-bit builds.
@TysonAndre TysonAndre requested a review from iluuu1994 February 5, 2022 16:50
Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

Thanks. I'll need to start using -Werror.

@TysonAndre TysonAndre merged commit 7b90ebe into php:master Feb 5, 2022
@TysonAndre TysonAndre deleted the zend_operators-signed-compare branch February 5, 2022 19:22
@TysonAndre
Copy link
Contributor Author

Thanks. I'll need to start using -Werror.

My original PR summary/title was wrong - this was using export CFLAGS='-Wall -Wextra -Werror' for local development of a PECL using zend_operators.h - the -Werror turns the warnings into errors, the -Wall -Wextra were adding the extra warnings.

@iluuu1994
Copy link
Member

@TysonAndre Ah I see. I thought I missed the error in the large compilation output (happened before).

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.

2 participants