On Wed, 15 Nov 2023 19:27:35 GMT, Phil Race <[email protected]> wrote:
>> Harshitha Onkar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> local var reused
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/ApplicationDelegate.m line
> 417:
>
>> 415: if (getenv("AWT_DISABLE_NSDELEGATE_SECURE_SAVE") != NULL) {
>> 416: supportsSecureState = NO;
>> 417: }
>
> I think that (and in the other case), it is best to read the env. var only
> once, not keep checking it on ever call to this method. Theoretically if
> someone used putenv/setenv (yes they'd need native code), it would cause us
> to change the answer.
>
> Yes, this makes the code a bit more complicated but it will be safer.
> maybe you could maybe use static locals
>
> <pre>
> (BOOL)applicationSupportsSecureRestorableState:(NSApplication *)app {
> static BOOL checked = NO;
> static BOOL supportsSecureState = YES;
> if (checked == NO) {
> checked = YES;
> if (getenv("AWT_DISABLE_NSDELEGATE_SECURE_SAVE") != NULL) {
> supportsSecureState = NO;
> }
> return supportsSecureState;
> }
>
> </pre>
@prrace I think using static variables to read env variables only once is a
good idea since I see `applicationSupportsSecureRestorableState()` being called
every time the window loses focus.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16569#discussion_r1397789526
