RFR (L/S) manual cleanup of white space issues (8057109)

[serguei.spitsyn at oracle.com]

On 9/9/14 7:25 AM, Daniel D. Daugherty wrote:
> On 9/8/14 6:17 PM, serguei.spitsyn at oracle.com wrote:
>> Nice cleanup!
>> It looks good.
>
> Thanks!
>
>
>> Some minor comments below.
>>
>>
>> On 9/8/14 3:01 PM, Daniel D. Daugherty wrote:
>>> Updated after Fred's code review:
>>>
>>> Here is the URL for the (L)arge webrev:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8057109-webrev/1-jdk9-hs-rt/
>>>
>>> Here is the URL for the (S)mall webrev which was generated with a 
>>> version
>>> of webrev that ignores all whitespace changes:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8057109-webrev/1-jdk9-hs-rt-no_ws/
>>>
>>> Dan
>>>
>>>
>>> On 9/7/14 1:13 PM, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> I have the fix for the following bug ready for JDK9 RT_Baseline:
>>>>
>>>>     JDK-8057109 manual cleanup of white space issues prior to 
>>>> Contended
>>>>                 Locking reorder and cache line bucket
>>>> https://bugs.openjdk.java.net/browse/JDK-8057109
>>>>
>>>> This is both a (L)arge and a (S)mall review! Here is the URL for the
>>>> (L)arge webrev:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8057109-webrev/0-jdk9-hs-rt/
>>
>> src/os/solaris/vm/os_solaris.cpp
>>    4022   assert((chainedsigactions != (struct sigaction *)NULL) && (preinstalled_sigs != (int *)NULL), "signals not yet initialized");
>>    The line above is worth to split as it is done for 4032-4033.
>
> Fixed, but I did not do an additional pass to look for more
> assert() calls that need reformatting.

Thanks!
I did not expect additional pass.

>
>
>>
>>
>> src/os/windows/vm/os_windows.cpp:
>>   196       *(strrchr(home_dir, '\\')) = '\0';  // get rid of \jvm.dll
>>   197       pslash = strrchr(home_dir, '\\');
>>   198       if (pslash != NULL) {
>>   199         *pslash = '\0';                 // get rid of \{client|server}
>>   200         pslash = strrchr(home_dir, '\\');
>>   201         if (pslash != NULL) {
>>   202           *pslash = '\0';             // get rid of \bin
>>   203         }
>>   Minor: It is better to align the comments.
>
> Fixed, but I did not do an additional pass to look for more
> comments that need aligning.


I did not expect additional pass.

Thanks!
Serguei

>
> Dan
>
>
>>
>>
>> Thanks,
>> Serguei
>>
>>>>
>>>> Since these are mostly white space fixes, I recommend the udiff links
>>>> instead of the frames links. Of course, some folks prefer the patch 
>>>> file
>>>> itself for white space fixes.
>>>>
>>>> Here is the URL for the (S)mall webrev which was generated with a 
>>>> version
>>>> of webrev that ignores all whitespace changes:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8057109-webrev/0-jdk9-hs-rt-no_ws/
>>>>
>>>> Again, I recommend the udiff links since that's the fastest way to see
>>>> that webrev_no_ws found minimal non-white space changes. The patch 
>>>> file
>>>> for this webrev shows all the changes.
>>>>
>>>> These manual changes were motivated by various review comments made 
>>>> for
>>>> earlier fixes in the Contended Locking project. During those 
>>>> reviews, I
>>>> deferred making some of the requested format changes to this round of
>>>> manual changes. In most cases, the reviewers posted comments with one
>>>> or two examples of things they did not like and I've swept all 17 
>>>> files
>>>> that we've targeted for cleanup in the Contended Locking project 
>>>> looking
>>>> for those style problems and fixing them. I've made a total of 12 
>>>> passes
>>>> over the 17 files, but I cannot say that I've caught everything. Most
>>>> stuff that annoys people is fixed, but it's very difficult to catch 
>>>> and
>>>> fix them all without a real parser based solution.
>>>>
>>>> Thanks, in advance, for any comments, questions or suggestions.
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>>
>>>
>>
>