On Mon, 2 Oct 2023 07:57:57 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
>> Prepares java.security.jgss for the permissive- compiler switch by
>>
>> - Adding scopes so goto doesn't jump over unitialized locals in sspi.cpp
>> - Adding a static modifier to a mismatched method declaration in
>> NativeCreds.c, as the definition is static
>
> src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp line 39:
>
>> 37:
>> 38: #include <windows.h>
>> 39: #include <cstdlib>
>
> This seems unrelated; my MSVC doesn't complain about these headers.
> The C form is still preferred, even in our CPP code. Can we leave this change
> out?
This was just a stylistic change, I'll leave it out
> src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp line 372:
>
>> 370: SEC_WCHAR* value = new SEC_WCHAR[len + 1];
>> 371:
>> 372: {
>
> This is ugly. I'm not a fan of braces appearing in the middle of the code for
> no apparent reason.
>
> [This SO
> question](https://stackoverflow.com/questions/31513798/error-jump-to-label-foo-crosses-initialization-of-bar)
> states that we can fix the compilation errors by splitting inline
> initialization into definition + assignment. I think I'd prefer that approach.
I agree that it's ugly, but at the time I couldn't think of another way to
solve the issue. By any chance, why does splitting it out into separate
declaration assignment work? Last I remember, it still jumps over the local
even when split up like this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15996#discussion_r1342399094
PR Review Comment: https://git.openjdk.org/jdk/pull/15996#discussion_r1342401153
