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

[Daniel D. Daugherty]

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.


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

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