Review request 8008310 - Some adjustments needed to, minimal VM warnings and errors for unsupported command line options

[David Holmes]

Thanks Joe. Good to go.

David

On 5/03/2013 1:11 AM, JOSEPH PROVINO wrote:
> latest webrev is here:
> http://cr.openjdk.java.net/~jprovino/8008310/webrev.04
>
> joe
>
> On 3/1/2013 9:31 AM, JOSEPH PROVINO wrote:
>>
>> On 3/1/2013 1:34 AM, David Holmes wrote:
>>> On 1/03/2013 4:54 AM, JOSEPH PROVINO wrote:
>>>> Latest webrev is here:
>>>> http://cr.openjdk.java.net/~jprovino/8008310/webrev.03
>>>
>>> One lingering glitch:
>>>
>>> 3273   if (DumpSharedSpaces || RequireSharedSpaces) {
>>> 3274     jio_fprintf(defaultStream::error_stream(),
>>> 3275       "Shared spaces are not supported in this VM\n");
>>> 3276     return JNI_ERR;
>>> 3277   }
>>> 3278   if (UseSharedSpaces || PrintSharedSpaces) {
>>> 3279     warning("Shared spaces are not supported in this VM");
>>> 3280   }
>>> 3281   FLAG_SET_DEFAULT(DumpSharedSpaces, false);
>>> 3282   FLAG_SET_DEFAULT(RequireSharedSpaces, false);
>>> 3283   FLAG_SET_DEFAULT(PrintSharedSpaces, false);
>>>
>>> The last three lines only need to be these two lines:
>>>
>>> FLAG_SET_DEFAULT(UseSharedSpaces, false);
>>> FLAG_SET_DEFAULT(PrintSharedSpaces, false);
>>>
>>> and they can move inside the if.
>>
>> Okay, I'll make those changes.
>>
>> joe
>>
>>>
>>> DumpSharedSpaces and RequireSharedSpaces must already be false else
>>> we exit at #3276.
>>>
>>> David
>>> -----
>>>
>>>> joe
>>>>
>>>> On 2/27/2013 11:36 PM, David Holmes wrote:
>>>>> On 28/02/2013 3:41 AM, JOSEPH PROVINO wrote:
>>>>>>
>>>>>> On 2/26/2013 8:45 PM, David Holmes wrote:
>>>>>>> On 27/02/2013 2:59 AM, JOSEPH PROVINO wrote:
>>>>>>>>
>>>>>>>> On 2/25/2013 8:43 PM, David Holmes wrote:
>>>>>>>>> On 26/02/2013 1:58 AM, JOSEPH PROVINO wrote:
>>>>>>>>>> Latest webrev is here:
>>>>>>>>>> http://cr.openjdk.java.net/~jprovino/8008310/webrev.02
>>>>>>>>>>
>>>>>>>>>> - excluded filemap.cpp if CDS is 0.
>>>>>>>>>>
>>>>>>>>>> - confined changes for CDS to filemap.hpp.
>>>>>>>>>
>>>>>>>>> These changes are good - thanks.
>>>>>>>>>
>>>>>>>>> For arguments.cpp:
>>>>>>>>>
>>>>>>>>> 1076     FLAG_SET_DEFAULT(RequireSharedSpaces, false);
>>>>>>>>>
>>>>>>>>> If we execute this line then the flag is already false.
>>>>>>>>>
>>>>>>>>> Also, as discussed in email -Xshare:dump should probably be an
>>>>>>>>> error
>>>>>>>>> not a warning, but note that if left as a warning then this code:
>>>>>>>>>
>>>>>>>>> 2518     // -Xshare:dump
>>>>>>>>> 2519     } else if (match_option(option, "-Xshare:dump", &tail)) {
>>>>>>>>> 2520       FLAG_SET_CMDLINE(bool, DumpSharedSpaces, true);
>>>>>>>>> 2521       set_mode_flags(_int);     // Prevent compilation, which
>>>>>>>>> creates objects
>>>>>>>>>
>>>>>>>>> would also force us into intepreter mode, so you would still
>>>>>>>>> need to
>>>>>>>>> check INCLUDE_CDS here.
>>>>>>>>
>>>>>>>> If DumpSharedSpaces is changed to return JNI_ERR do I still need a
>>>>>>>> conditional here?
>>>>>>>
>>>>>>> ?? You would need the conditional to decide to return JNI_ERR.
>>>>>>
>>>>>> I meant do I need #if INCLUDE_CDS around set_mode_flags(_int);
>>>>>
>>>>> So I was assuming this was the only place you needed to check this,
>>>>> but you have to check for -XX:+DumpSharedSpaces as well. So if you
>>>>> left the -Xshare:dump code alone and later checked for
>>>>> DumpSharedSpaces and returned with JNI_ERR then it would not matter
>>>>> that the switch to interpreter mode had already been made.
>>>>>
>>>>> David
>>>>>
>>>>>> joe
>>>>>>
>>>>>>>
>>>>>>> David
>>>>>>>
>>>>>>>> joe
>>>>>>>>
>>>>>>>>>
>>>>>>>>> David
>>>>>>>>> -----
>>>>>>>>>
>>>>>>>>>> thanks.
>>>>>>>>>>
>>>>>>>>>> joe
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
>