RFR: 8320219: Actually resolve issues with goto labels in sspi [v7]
Weijun Wang
weijun at openjdk.org
Tue Jan 16 17:46:26 UTC 2024
On Wed, 10 Jan 2024 12:53:48 GMT, Julian Waters <jwaters at openjdk.org> wrote:
>> I regret not actually addressing the issues with the goto labels in https://github.com/openjdk/jdk/pull/15996, where initialization of locals in sspi were jumped over by gotos to a certain label. I changed the initializations into split declarations and assignments in https://github.com/openjdk/jdk/pull/15996, but this is simply a hack and does not address the real issue of gotos jumping over locals. I've as such fixed the issues with them properly this time, by simply deleting the labels and duplicating the code where they're used. As mentioned, this unfortunately does increase duplicate code, but is the cleanest solution I could come up with for the labels
>
> Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix the remaining case in sspi.cpp
Thanks a lot for your patience. I've added some comments.
src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp line 382:
> 380:
> 381: execution:
> 382:
Remove this empty line to be consistent with the `err:` branch.
src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp line 422:
> 420: gss_name_struct* name = new gss_name_struct;
> 421: if (name == nullptr) {
> 422: goto err;
Go back? This looks more evil to me. Can we just `delete` and `return` here?
src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp line 538:
> 536: goto execution;
> 537: cleanup:
> 538: static_cast<void>(0);
What is this for?
src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp line 543:
> 541: delete[] fullname;
> 542: }
> 543: goto finish;
Instead of `goto finish`, can we `return` here?
src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp line 580:
> 578: exported_name->value = buffer;
> 579: result = GSS_S_COMPLETE;
> 580: goto cleanup;
Can we duplicate `delete[] fullname` and `return` here?
-------------
PR Review: https://git.openjdk.org/jdk/pull/16682#pullrequestreview-1824364216
PR Review Comment: https://git.openjdk.org/jdk/pull/16682#discussion_r1453756600
PR Review Comment: https://git.openjdk.org/jdk/pull/16682#discussion_r1453757389
PR Review Comment: https://git.openjdk.org/jdk/pull/16682#discussion_r1453762269
PR Review Comment: https://git.openjdk.org/jdk/pull/16682#discussion_r1453763075
PR Review Comment: https://git.openjdk.org/jdk/pull/16682#discussion_r1453764065
More information about the security-dev
mailing list