Looks good.
There is also a possible buffer overrun some lines above:
357 char* posbin = strstr(execname, "/bin/java");
358 if (posbin != NULL) {
359 memcpy(filepath, execname, posbin - execname); // not include
trailing '/'
360 filepath[posbin - execname] = '\0';
depending on the execname length.
Also, the strstr() is probably wrong since it also fires for paths which
have "/bin/java" somewhere in the middle, however unlikely that my be.
I am fine with your change as it is though.
Cheers, Thomas
On Thu, Oct 24, 2019 at 4:16 PM Simon Tooke <[email protected]> wrote:
> Hello,
>
> While reviewing uses of strtok() with an eye to moving to strtok_r(), I
> came accross an inifinite loop in the macOS agent code, but one that has
> probably never been executed. In the interests of not having even
> potential loops, I've file a bug and have a PR to submit. My patch
> removes the inifinite loop and switches to strtok_r(). The switch to
> the reentrant version is not required in this use case but I include it
> so that this code doesn't show up on future scans for strtok() usage.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8232973
>
> Webrev: http://cr.openjdk.java.net/~stooke/webrevs/jdk-8232973-jdk/00/
>
> Are there any concerns?
>
> Thanks,
>
> -Simon
>
>
>
>
