On Mon, 11 Mar 2024 18:36:57 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
> > If you really want to require an exact match for jspawnhelper, then these 4 > > numbers aren't necessarily enough, but a rather arbitrarily chosen > > approximation. > > I think for our purposes, a version number quadruplet is enough to provide > basic level of safety for jspawnhelper protocol updates across JDK versions, > without version checking code being a safety problem itself. > > Putting in the full version string would raise more questions, at least for > me. For example, are we guaranteed that version string always fits the > argument line? Probably so. Would we get some interesting (Unicode?) symbols > in version string that would break somewhere along the way? Would we need to > dynamically allocate the buffer for arguments, instead of allocating enough > to hold the version _integers_? Would we make a mistake while doing so? Etc. > > All in all, it feels better to silently accept some version mismatches not > captured by the version quadruplet, rather than fail trying to do a perfect > match. Legal characters for the version string are defined in https://openjdk.org/jeps/223 and verified by configure. There is no risk of weird unicode being included. If the full version string is too long, you could consider using `VERSION_SHORT`, which drops the $OPT, and the $BUILD part. I don't think we need to dynamically allocate the buffer. The required buffer length is known at compile time as the length of the expected version string (which we get as a command line macro) +1. If the user supplies more, we don't need to compare the rest of the string as we already know it's not equal. To me, hardcoding of 4 version digits raises more questions. We spend unnecessary time and effort specifically serializing and de-serializing 4 numbers, always maintaining the correct order. To me that's brittle code. If the version elements, or how we use them, were to change in the future, then this code needs to be carefully revisited, whereas if we just used a standard version string, it would continue to work. ------------- PR Comment: https://git.openjdk.org/jdk/pull/18204#issuecomment-1989320103
