On 7/14/20 15:55, [email protected] wrote:
Hi Chris and Alex,

I agree the last occurrence of "/bin/" is better than the first.
But I wonder if it makes sense to check all occurrences.

Forgot to suggest to re-balance the comment to make the first line a little shorter:
 381       // Last effort. Look for bin directory in path. This is useful when attaching to a java process
 382       // that was launched with a JDK tool other than "java".

Thanks,
Serguei


Thanks,
Serguei


On 7/14/20 15:14, Alex Menkov wrote:
Yes, you are right.
This is not a function from strings.h

Ok, you can leave strstr (and keep in mind that the path can't contain "/bin/" other than jdk's bin) or implement the functionality. It should be something simple like

static const char* rstrstr(const char *str, const char *sub) {
  const char *result = NULL;
  for (const char *p = strstr(str, sub); p != NULL; p = strstr(p + 1, sub)) {
    result = p;
  }
  return result;
}

--alex

On 07/14/2020 13:43, Chris Plummer wrote:
Actually it's not so easy. I don't see any other references to strrstr in our source. When I reference strstr, it gives a warning because it's not declared. The only man page I can find says to include sstring2.h, but this file does not exist. It also says to link with -lsstrings2.

Chris

On 7/14/20 1:37 PM, Chris Plummer wrote:
Ok. I'll change both references to use strrstr.

thanks,

Chris

On 7/14/20 1:11 PM, Alex Menkov wrote:
Hi Chris,

I think it would be better to use strrstr to correctly handle paths like
/something/bin/jdk/bin/jhsdb

And I'd updated
358   char* posbin = strstr(execname, "/bin/java");
to use strrstr as well

--alex

On 07/14/2020 12:01, Chris Plummer wrote:
Ping!

On 7/6/20 9:31 PM, Chris Plummer wrote:
Hello,

Please help review the following:

http://cr.openjdk.java.net/~cjplummer/8248879/webrev.00/index.html
https://bugs.openjdk.java.net/browse/JDK-8248879

The description of the problem and the fix are both in the CR.

thanks,

Chris










Reply via email to