<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Couple of minor comments in
ostream.cpp:<br>
<br>
1) <br>
<span class="new"> 479 char time_msg[256];<br>
480 char time_str[64];<br>
481 char renamed_file_name[256];<br>
<br>
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.<br>
<br>
In the same file, there's <br>
static const int buffer_length = 32;<br>
in outputStream::date_stamp().<br>
<br>
I'm thinking you can have a #define for the size for time_str
and use the same #define in </span><br>
<span class="new"><span class="new">outputStream::date_stamp().<br>
<br>
2)<br>
int jio_snprintf(char *str, size_t count, const char *fmt,
...)<br>
<br>
The return value of jio_snprintf() isn't checked. <br>
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.<br>
<br>
3)<br>
</span></span><br>
<span class="new"><span class="new"><span class="new">516 if
(access(renamed_file_name, NOT_WINDOWS(F_OK)
WINDOWS_ONLY(0)) == 0) {</span>
<br>
</span></span><br>
<span class="new"><span class="new">543 if
(access(renamed_file_name, NOT_WINDOWS(F_OK) WINDOWS_ONLY(0))
== 0) {<br>
<br>
You can use ERROR_SUCCESS instead of 0 for the WINDOWS_ONLY
part.<br>
<br>
Calvin<br>
</span></span><br>
On 8/15/2013 8:35 AM, Yumin Qi wrote:<br>
</div>
<blockquote cite="mid:520CF52B.6050000@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
Hi, <br>
<br>
Can I have your review for this small changes?<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev00/">http://cr.openjdk.java.net/~minqi/7164841/webrev00/</a><br>
<br>
This is for a enhancement to add head/tail message to the
logging files to assist reading GC output.<br>
1. modified prompt message if invalid arguments used for log
rotating;<br>
2. add time and file name message to log file head/tail.<br>
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<br>
On Windows, there is no F_OK (existing test) definition,
F_OK is defined as "0" and for _access of VC++, it just describes:<br>
<br>
<table style="border-collapse: collapse; padding: 0px; width:
1306px; border: 1px solid rgb(187, 187, 187); color: rgb(0, 0,
0); font-family: 'Segoe UI', 'Lucida Grande', Verdana, Arial,
Helvetica, sans-serif; font-size: 13px; font-style: normal;
font-variant: normal; font-weight: normal; letter-spacing:
normal; line-height: 17px; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: auto; word-spacing: 0px; -webkit-text-stroke-width:
0px;">
<tbody>
<tr>
<th style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; background-color: rgb(237, 237,
237); color: rgb(112, 112, 112); text-align: left;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;"><span class="parameter" style="font-style:
italic;">mode</span><span
class="Apple-converted-space"> </span>value</p>
</th>
<th style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; background-color: rgb(237, 237,
237); color: rgb(112, 112, 112); text-align: left;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">Checks file for</p>
</th>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">00</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">Existence only</p>
</td>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">02</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">Write-only</p>
</td>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">04</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">Read-only</p>
</td>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">06</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187); margin:
10px; padding: 10px 8px; color: rgb(42, 42, 42);
vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px; line-height:
18px;">Read and write</p>
</td>
</tr>
</tbody>
</table>
<br>
<a moz-do-not-send="true"
href="http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx">http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx</a><br>
The definition are consistent with unistd.h. <br>
<br>
Test: JPRT and jtreg.<br>
<br>
Thanks<br>
Yumin<br>
</blockquote>
<br>
</body>
</html>