RFR: 8187570: Comparison between pointer and char in MethodMatcher::canonicalize

[Erik Österlund]

Looks good.

/Erik

On 2017-09-15 11:24, Erik Helin wrote:
> Hi all,
>
> when I compiled with gcc 7.1.1 it warned me about the following code:
>
> bool MethodMatcher::canonicalize(char * line, const char *& error_msg) {
>   char* colon = strstr(line, "::");
>   bool have_colon = (colon != NULL);
>   if (have_colon) {
>     // Don't allow multiple '::'
>     if (colon + 2 != '\0') {
>
> The problem is that colon is a pointer, so colon + 2 is a pointer, and 
> then colon + 2 is compared '\0', which is a char :/
>
> Anyways, I think Yasumasa also spotted this issue a while back, but I 
> couldn't find a patch for it, so I quickly whipped one up:
>
> http://cr.openjdk.java.net/~ehelin/8187570/00/
>
> --- old/src/hotspot/share/compiler/methodMatcher.cpp    2017-09-15 
> 10:43:49.430656504 +0200
> +++ new/src/hotspot/share/compiler/methodMatcher.cpp    2017-09-15 
> 10:43:49.102654877 +0200
> @@ -96,7 +96,7 @@
>    bool have_colon = (colon != NULL);
>    if (have_colon) {
>      // Don't allow multiple '::'
> -    if (colon + 2 != '\0') {
> +    if (colon[2] != '\0') {
>        if (strstr(colon+2, "::")) {
>          error_msg = "Method pattern only allows one '::' allowed";
>          return false;
>
> I was a little bit afraid of what would happen if line (the parameter 
> to MethodMatcher::canonicalize) isn't properly null terminated, so I 
> checked the callers, and it seems like all callers property null 
> terminate the `line` argument.
>
> Yasumasa, you want to be author and/or contributor of this patch?
>
> Thanks,
> Erik
>
> PS. First webrev and patch created with a consolidated forest, seems 
> to be working fine :)