RFR: 7164841: Improvements to the GC log file rotation

[Calvin Cheung]

Couple of minor comments in ostream.cpp:

1)
   479   char time_msg[256];
  480   char time_str[64];
  481   char renamed_file_name[256];

I think you can replace 256 with MAX_PATH. However, we define MAX_PATH 
as (2 * K) in unix platforms. So it maybe too large of a buffer for this 
purpose. On windows, I think it is defined as 256.

In the same file, there's
     static const int buffer_length = 32;
in outputStream::date_stamp().

I'm thinking you can have a #define for the size for time_str and use 
the same #define in
outputStream::date_stamp().

2)
int jio_snprintf(char *str, size_t count, const char *fmt, ...)

The return value of jio_snprintf() isn't checked.
It could return -1 from calling _vsnprintf(). However, I don't see the 
return status being check in the existing code. So I'm not sure if it is 
a must check.

3)

516 if (access(renamed_file_name, NOT_WINDOWS(F_OK) WINDOWS_ONLY(0)) == 
0) {

543 if (access(renamed_file_name, NOT_WINDOWS(F_OK) WINDOWS_ONLY(0)) == 0) {

You can use ERROR_SUCCESS instead of 0 for the WINDOWS_ONLY part.

Calvin

On 8/15/2013 8:35 AM, Yumin Qi wrote:
> Hi,
>
>   Can I have your review for this small changes?
> http://cr.openjdk.java.net/~minqi/7164841/webrev00/ 
> <http://cr.openjdk.java.net/%7Eminqi/7164841/webrev00/>
>
>    This is for a enhancement to add head/tail message to the logging 
> files to assist reading GC output.
>    1. modified prompt message if invalid arguments used for log rotating;
>    2. add time and file name message to log file head/tail.
>    3. for easily identify which log file is current, use file name 
> like <filename>.n.current, after it reaches maximum size, rename it to 
> <filename>.n
>         On Windows, there is no F_OK (existing test) definition, F_OK 
> is defined as "0" and for _access of VC++, it just describes:
>
> modevalue
>
> 	
>
> Checks file for
>
> 00
>
> 	
>
> Existence only
>
> 02
>
> 	
>
> Write-only
>
> 04
>
> 	
>
> Read-only
>
> 06
>
> 	
>
> Read and write
>
>
> http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx
> The definition are consistent with unistd.h.
>
>     Test: JPRT and jtreg.
>
>    Thanks
>    Yumin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130816/df7c27b9/attachment-0001.html