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

[JOSEPH PROVINO]

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
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>