On Fri, 17 Nov 2023 14:13:58 GMT, Thomas Stuefe <stu...@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

Reply via email to