RFR: 7164841: Improvements to the GC log file rotation
Calvin Cheung
calvin.cheung at oracle.com
Sat Aug 17 01:26:25 UTC 2013
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: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130816/df7c27b9/attachment.htm>
More information about the hotspot-gc-dev
mailing list