On Wed, 10 Jan 2024 12:53:48 GMT, Julian Waters <jwat...@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
