On Tue, 4 Oct 2022 05:44:27 GMT, David Holmes <dhol...@openjdk.org> wrote:
> > JNU_ThrowByNameWithStrerror > > JNU_ThrowByNamePerror > > JNU_ThrowIOExceptionWithIOError > > The problem with this kind of naming is that the user of the API for which > these functions are then called, should not have to know anything about the > origins of the actual error code/string. The call-sites really do want to say > `ThrowXXXWithLastError`. That's a good point for the first 2, but I do feel like it would be helpful to specify the kind of error the utilities that are more specialized (NET_ThrowNewWithLastError and JNU_ThrowIOExceptionWithLastError for instance), since both use very different subsystems on Windows and POSIX, which means they can be segregated accordingly. In my opinion the specific error type could be specified after "Last" instead of replacing it as a compromise. The tricky part comes with the first 2 (JNU_ThrowByNameWithLastError and JNU_ThrowByNameWithMessageAndLastError), which are for general use, but their original implementations fell back to the initial problematic mixing of WIN32 and errno errors, and leaving their names unchanged might just result in more ambiguity at the callsites they're used at. I also don't think I came up with particularly good names for them though, and honestly I'm at a bit of a loss as to what should be done with them at this point, hopefully more reviews can come in wit h some insight on this end. > Maybe I'm wrong to believe that they can be ignorant of the details but the > level of abstraction seems wrong to me. The issue with that was Thomas's initial concerns that you can't really use the last error like this without at least some knowledge about the origin and nature of the actual error itself (WIN32 completely bypassing errno on Windows, and APIs on POSIX arbitrarily using it sometimes but using a return value instead to signal an error other times). I did try to address that by making it a little more specific with this patch, but it still seems there's a better way to do it than this... I'll try to address the other reviews in the meantime though, before coming back to this. ------------- PR: https://git.openjdk.org/jdk/pull/9870
