> On Aug 10, 2023, at 15:29, Mark Thomas <ma...@apache.org> wrote:
>
> On 10/08/2023 04:04, li...@apache.org <mailto:li...@apache.org> wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>> lihan pushed a commit to branch main
>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>> <https://gitbox.apache.org/repos/asf/tomcat.git>
>> The following commit(s) were added to refs/heads/main by this push:
>> new 6353e78658 Minor optimization
>
> -1. Veto. Please fix / revert. Details in-line.
>
> This veto also applies to the back-ports of this change.
>
>> 6353e78658 is described below
>> commit 6353e78658ea09c5056943901877f0b6ed31faae
>> Author: lihan <li...@apache.org>
>> AuthorDate: Wed Aug 9 16:42:11 2023 +0800
>> Minor optimization
>> ---
>> java/org/apache/catalina/mapper/Mapper.java | 20 ++++++++++++--------
>> 1 file changed, 12 insertions(+), 8 deletions(-)
>> diff --git a/java/org/apache/catalina/mapper/Mapper.java
>> b/java/org/apache/catalina/mapper/Mapper.java
>> index 9a3cfd833f..b1cfa8033b 100644
>> --- a/java/org/apache/catalina/mapper/Mapper.java
>> +++ b/java/org/apache/catalina/mapper/Mapper.java
>> @@ -1337,16 +1337,18 @@ public final class Mapper {
>> len = end - start;
>> }
>> for (int i = 0; (i < len) && (result == 0); i++) {
>> - if (c[i + start] > compareTo.charAt(i)) {
>> + char nameChar = c[i + start];
>> + char compareToChar = compareTo.charAt(i);
>> + if (nameChar > compareToChar) {
>> result = 1;
>> - } else if (c[i + start] < compareTo.charAt(i)) {
>> + } else if (nameChar < compareToChar) {
>> result = -1;
>> }
>> }
>> if (result == 0) {
>> - if (compareTo.length() > (end - start)) {
>> + if (len > (end - start)) {
>
> compareTo.length() is NOT equivalent to len at this point since len may be
> modified at the start of the method.
Oops. I'll fix it later.
Han
>
>> result = -1;
>> - } else if (compareTo.length() < (end - start)) {
>> + } else if (len < (end - start)) {
>
> Same issue here.
>
>> result = 1;
>> }
>> }
>> @@ -1366,16 +1368,18 @@ public final class Mapper {
>> len = end - start;
>> }
>> for (int i = 0; (i < len) && (result == 0); i++) {
>> - if (Ascii.toLower(c[i + start]) > Ascii.toLower(compareTo.charAt(i))) {
>> + int nameLower = Ascii.toLower(c[i + start]);
>> + int compareLower = Ascii.toLower(compareTo.charAt(i));
>> + if (nameLower > compareLower) {
>> result = 1;
>> - } else if (Ascii.toLower(c[i + start]) <
>> Ascii.toLower(compareTo.charAt(i))) {
>> + } else if (nameLower < compareLower) {
>> result = -1;
>> }
>> }
>> if (result == 0) {
>> - if (compareTo.length() > (end - start)) {
>> + if (len > (end - start)) {
>
> Same issue here.
>
>> result = -1;
>> - } else if (compareTo.length() < (end - start)) {
>> + } else if (len < (end - start)) {
>
> Same issue here.
>
>> result = 1;
>> }
>> }
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> <mailto:dev-unsubscr...@tomcat.apache.org>
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> <mailto:dev-h...@tomcat.apache.org>
