RFR: 8320219: Actually resolve issues with goto labels in sspi [v3]

Julian Waters jwaters at openjdk.org
Fri Nov 17 14:36:33 UTC 2023


On Fri, 17 Nov 2023 14:13:58 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> > > Can you please describe the problem you are trying to solve, and why you think it is worth solving. You describe the thought process you went through while doing the patch and possibly some other, older patch. As it is, reading the PR text or the issue description is confusing and requires reviewers to dig through comment threads on older issues to parse it.
> > 
> > 
> > I've changed to description for both to (hopefully) be more descriptive and clear. Sorry for the hassle, I'm not particularly good with words
> 
> Somewhat better, but not by much, sorry :-)
> 
> This patch is work to review, since it touches security-relevant stuff and because mentally verifying that all the code paths you change are equivalent to what happened before is real work. It is probably more work than doing the patch.
> 
> Hence my question, what is the motivation, is there a bug that needs fixing? Or is it just that you dislike these gotos? Is it just to get Windows to build with GCC? Sorry, I did not follow your work, so I don't have a context.

This is a cleanup patch that follows the older one that enabled security to compile with permissive-, the problem with that one is that simply changing the locals to a split declaration and assignment is a bit of an ugly hack, since the locals then have undefined values when allocated. gcc already compiles fine with the old patch, this new one is not needed for either compiler (MSVC or gcc) to compile without errors. I felt like I had the responsibility to clean up what is essentially a hack just to get it to compile with permissive- on MSVC. This is also partially inspired by Phil's rejection of the old patch in https://github.com/openjdk/jdk/pull/15096 :/

I don't have anything against gotos, nor do I dislike them :)

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16682#issuecomment-1816538037



More information about the security-dev mailing list