On Tue, 11 Jun 2024 19:01:45 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Thanks I'll go through the above comments and update - some of the changes I >> see are unnecessary and from when I was trying migrating to callAs, and >> doPrivileged, and yes they can be simpler. >> >> On the allowSecurityMananger check: >> If we check in e.g. RMIConnectionImpl constructor, and if SM is allowed we >> assign an ACC. >> Later, we know if we have a non-null ACC then SM is allowed. Are you saying >> we should then check allowSecurityManager again? > > Not "then check" but "first check", as shown in my example above. I > understand you can infer whether SM is allowed by looking at the values if > `acc` and `subject`, but IMO it's not worth doing. The check is very light > and this coding style makes sure the old code and new code are cleanly > separated. It's easy to code because you don't modify existing code and only > add new code in else blocks. In the near future when we finally remove the SM > it will also be simple to remove old code. I'm trying that out in this update. It looks OK 8-) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1638932678
