AW: AW: [11u] RFR: 8266713: [AIX] Build failure after 11u backport of JDK-8247753

[Doerr, Martin]

Hi Severin,

thanks for the update. Looks correct to me, now.

> I'm not sure I understood your 'haystack_len - needle_len' comment.
You don’t have to check k < haystack_len in the inner loop anymore.
But I agree with that performance shouldn’t be an issue. I just like shorter conditions. ��

Best regards,
Martin


Von: Severin Gehwolf <sgehwolf at redhat.com>
Datum: Montag, 10. Mai 2021 um 21:27
An: Doerr, Martin <martin.doerr at sap.com>, jdk-updates-dev <jdk-updates-dev at openjdk.java.net>
Cc: Baesken, Matthias <matthias.baesken at sap.com>, Stuefe, Thomas <thomas.stuefe at sap.com>
Betreff: Re: AW: [11u] RFR: 8266713: [AIX] Build failure after 11u backport of JDK-8247753
Hi Martin,

On Mon, 2021-05-10 at 17:51 +0000, Doerr, Martin wrote:
> Hi Severin,
>
> thank you for addressing this issue!
> I like the idea of supporting strcasestr on AIX. I’m ok with having it
> in the same file for now.
>
> Seems like you wanted to initialize i, not k.
> Besides that, the algorithm looks correct to me.
> The comparisons in the loop would be simpler when using loop limit
> haystack_len - needle_len, but I leave you free to decide. We don’t
> have to optimize performance.

Thanks for the review! Updated webrev:
https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8266713/03/webrev/

I'm not sure I understood your 'haystack_len - needle_len' comment.
Anyway, performance shouldn't really be an issue here.

Thanks,
Severin

>
> Von:jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> im Auftrag
> von Severin Gehwolf <sgehwolf at redhat.com>
> Datum: Montag, 10. Mai 2021 um 15:26
> An: jdk-updates-dev <jdk-updates-dev at openjdk.java.net>
> Cc: Baesken, Matthias <matthias.baesken at sap.com>, Stuefe, Thomas <
> thomas.stuefe at sap.com>
> Betreff: [11u] RFR: 8266713: [AIX] Build failure after 11u backport of
> JDK-8247753
> Hi,
>
> Please review this AIX build fix for OpenJDK 11u. The JDK 11u backport
> of JDK-8247753 included a call to strcasestr for case-insensitive
> comparison of the set XDG_CURRENT_DESKTOP environment variable.
> Unfortunately, strcasestr is a GNU extension and isn't available on
> AIX. The proposed fix is to implement a version of strcasestr AIX-local
> in java_props_md.c. Note that this is a JDK 11u-specific bug since the
> backport of JDK-8247753 is different to the version in later JDKs where
> this comparison is not done in native code, but in Java.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8266713
> webrev:
> https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8266713/02/webrev/
>
> AIX failed to build prior this patch and builds with this patch after.
> Manual sanity checks for the strcasestr() impl.
>
> Thoughts?
>
> Thanks,
> Severin