> On Jun 5, 2016, at 5:50 PM, David Holmes <[email protected]> wrote:
>
>
> Taking an initial look at the VM changes ...
>
> I may be missing the full context but you seem to be checking only that some
> prefix matches the expected property format, not that the entire value is
> valid eg. if passed jdk.module.patch.1junk it will match
>
-Djdk.module.patch.1junk=<arg> is expected not match and should not be ignored.
Hence it should be set in the system property.
155 ((strncmp(prop_end, "patch.", 6) == 0) && isdigit(prop_end[6]))) {
Harold - is it intentional? I assume we want to match the suffix if it’s a
number.
> You use NEW_C_HEAP_ARRAY in places but that will abort the VM on failure,
> whereas you return error codes for other failure modes. For consistency use
> NEW_C_HEAP_ARRAY_RETURN_NULL and return an error on NULL. (This seems a
> pre-existing flaw.)
>
> There seem to be far too many string literals for the various jdk.module.*
> forms and lots of hard-coded string lengths. It would be nice if that could
> be cleaned up using #defines, or string constant variables, so that all the
> potential property names can be located in one place.
I agree.
>
> Are there any tests in hotspot/test/* that will require changes for this?
Not in the hotspot open repo.
Mandy
