Skip to content

Enforce literals in certain macros #10111

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
Jan 4, 2023
Merged

Conversation

morrisonlevi
Copy link
Contributor

@morrisonlevi morrisonlevi commented Dec 15, 2022

The zend_string_equals_literal_ci and zend_string_equals_literal
macros are designed only for literals. This is a standards compliant
trick to ensure they are literals. It relies on the fact that two
adjacent string literals are concatenated. For example, these are the
same:

"Hello" " world"
"Hello world"

Since literals are ensured, use sizeof(...) - 1 instead of strlen.

I spot checked usages visually, but running the CI in case it finds any
non-literal uses.

These macros are designed only for literals. This is a
standards compliant trick to ensure they are literals.
@morrisonlevi
Copy link
Contributor Author

The Travis CI job failure is concerning, though I don't think it's related to this PR. It's short, so I'll just copy its contents:

--TEST--
Test file_exists() function : usage variations
--CREDITS--
Dave Kelsey <[email protected]>
--FILE--
<?php
echo "*** Testing file_exists() : usage variations ***\n";

var_dump(file_exists(false));
var_dump(file_exists(''));
var_dump(file_exists(' '));
var_dump(file_exists('|'));
echo "Done";
?>
--EXPECT--
*** Testing file_exists() : usage variations ***
bool(false)
bool(false)
bool(false)
bool(false)
Done

Here's the diff:

========DIFF========
     *** Testing file_exists() : usage variations ***
     bool(false)
004+ bool(true)
     bool(false)
005- bool(false)
     Done
========DONE========
FAIL Test file_exists() function : usage variations [ext/standard/tests/file/file_exists_variation1.phpt] 
=====================================================================

@morrisonlevi morrisonlevi merged commit 0f4d37d into master Jan 4, 2023
@morrisonlevi morrisonlevi deleted the levim/cstr_literals branch January 4, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant