On 5/05/2016 12:18 PM, Yasumasa Suenaga wrote:
On 2016/05/05 10:52, David Holmes wrote:
+1 from me.
Thanks David!
I will sponsor this - currently submitting to JPRT.
Marcus will be a sponsor.
Sorry too late already done.
David
Thanks,
Yasumasa
Thanks,
David
On 4/05/2016 11:49
On 2016/05/05 10:52, David Holmes wrote:
+1 from me.
Thanks David!
I will sponsor this - currently submitting to JPRT.
Marcus will be a sponsor.
Thanks,
Yasumasa
Thanks,
David
On 4/05/2016 11:49 PM, Marcus Larsson wrote:
On 05/04/2016 03:43 PM, Yasumasa Suenaga wrote:
Thanks, Mar
+1 from me.
I will sponsor this - currently submitting to JPRT.
Thanks,
David
On 4/05/2016 11:49 PM, Marcus Larsson wrote:
On 05/04/2016 03:43 PM, Yasumasa Suenaga wrote:
Thanks, Marcus!
into something like
if (!success || values > SIZE_MAX) {
Also, I think you probably need a static_ca
On 05/04/2016 03:43 PM, Yasumasa Suenaga wrote:
Thanks, Marcus!
into something like
if (!success || values > SIZE_MAX) {
Also, I think you probably need a static_cast here:
201 _rotate_size = value;
I applied them to new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.
Thanks, Marcus!
into something like
if (!success || values > SIZE_MAX) {
Also, I think you probably need a static_cast here:
201 _rotate_size = value;
I applied them to new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.07/
Thanks,
Yasumasa
On 2016/05/04 21:38, Ma
Hi,
On 05/04/2016 02:29 PM, Yasumasa Suenaga wrote:
Hi David, Marcus,
Thank you for your comment.
I uploaded new webrev. Could you review it again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.06/
Can you join these two cases:
193 if (!success) {
194 break;
195 } else if (valu
Hi David, Marcus,
Thank you for your comment.
I uploaded new webrev. Could you review it again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.06/
Thanks,
Yasumasa
On 2016/05/04 15:49, Marcus Larsson wrote:
Hi,
On 05/04/2016 03:45 AM, David Holmes wrote:
On 4/05/2016 10:19 AM,
Hi,
On 05/04/2016 03:45 AM, David Holmes wrote:
On 4/05/2016 10:19 AM, Yasumasa Suenaga wrote:
Hi David, Marcus,
I would not have done that but instead used a temporary julong for the
parse result, then range check, then assign to the actual _rotate_size
etc with a cast.
Thanks.
However, I
On 4/05/2016 10:19 AM, Yasumasa Suenaga wrote:
Hi David, Marcus,
I would not have done that but instead used a temporary julong for the
parse result, then range check, then assign to the actual _rotate_size
etc with a cast.
Thanks.
However, I guess we will encounter error when we access to >
Hi David, Marcus,
I would not have done that but instead used a temporary julong for the parse
result, then range check, then assign to the actual _rotate_size etc with a
cast.
Thanks.
However, I guess we will encounter error when we access to > 2GB size file.
(Sorry, 4GB is incorrect.)
I d
On 3/05/2016 11:49 PM, Yasumasa Suenaga wrote:
This seems broken to me, we're passing &_rotate_size (a size_t*) to
atojulong() which takes a julong*. If sizeof(julong) > sizeof(size_t)
then that's a problem, no?
Thanks, Marcus!
Yes good catch. I would have hoped the compiler would have compla
Hi Marcus,
That's good, but then we shouldn't limit it to SIZE_MAX. The overflow check in
logFileOutput.cpp should be removed.
In Linux 32 bits system, we cannot open > 4GB size file because we do not pass
O_LARGEFILE.
(we use fopen(3))
In manpage [1], we should use _FILE_OFFSET_BITS rather
On 05/03/2016 03:49 PM, Yasumasa Suenaga wrote:
This seems broken to me, we're passing &_rotate_size (a size_t*) to
atojulong() which takes a julong*. If sizeof(julong) > sizeof(size_t)
then that's a problem, no?
Thanks, Marcus!
I changed type of _rotate_size and _current_size to julong in n
This seems broken to me, we're passing &_rotate_size (a size_t*) to atojulong()
which takes a julong*. If sizeof(julong) > sizeof(size_t) then that's a problem, no?
Thanks, Marcus!
I changed type of _rotate_size and _current_size to julong in new webrev:
http://cr.openjdk.java.net/~ysuenaga/
Hi,
On 05/03/2016 02:25 PM, David Holmes wrote:
Adding in the runtime team as they now own UL.
I've reviewed the change from a coding perspective - the question for
the UL owners is whether the change in semantics is appropriate:
previously the filesize was interpreted as a value in KB, wher
Adding in the runtime team as they now own UL.
I've reviewed the change from a coding perspective - the question for
the UL owners is whether the change in semantics is appropriate:
previously the filesize was interpreted as a value in KB, whereas now,
without a suffix, it is just bytes.
Tha
PING: Could you review and sponsor it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.04/
Thanks,
Yasumasa
On 2016/04/21 18:37, Yasumasa Suenaga wrote:
Hi David,
So it just registered with me that currently filesize is interpreted as a value
in KB. With this change it will b
Hi David,
So it just registered with me that currently filesize is interpreted as a value
in KB. With this change it will be in bytes - that means tests will need fixing
eg:
hotspot/test/serviceability/logging/TestLogRotation.java
Thanks!
I've fixed it in new webrev:
http://cr.openjdk.ja
Hi Yasumasa,
On 20/04/2016 7:15 PM, Yasumasa Suenaga wrote:
Hi David,
... on 32-bit size_t and julong are not the same size so we would
still need to ensure we don't specify a filesize that is greater than
SIZE_MAX on 32-bit.
Oh... I understood.
I've fixed and uploaded new webrev. Could you
Hi David,
... on 32-bit size_t and julong are not the same size so we would still need to
ensure we don't specify a filesize that is greater than SIZE_MAX on 32-bit.
Oh... I understood.
I've fixed and uploaded new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-815
On 20/04/2016 3:25 PM, Yasumasa Suenaga wrote:
Hi David,
> You still removed the size bounds checks:
>
> 190 size_t value = parse_value(value_str);
> 191 if (value == SIZE_MAX || value > SIZE_MAX / K) {
> 192 errstream->print_cr("Invalid option: %s must be in range
[0
Hi David,
> You still removed the size bounds checks:
>
> 190 size_t value = parse_value(value_str);
> 191 if (value == SIZE_MAX || value > SIZE_MAX / K) {
> 192 errstream->print_cr("Invalid option: %s must be in range [0,
"
> 193 SIZE_FORMAT "]"
Hi Yasumasa,
On 19/04/2016 11:50 PM, Yasumasa Suenaga wrote:
> Hi David,
>
> Thank you for your comment.
>
> I uploaded new webrev. Could you review again?
>http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.02/
You still removed the size bounds checks:
190 size_t value = parse
Hi David,
Thank you for your comment.
I uploaded new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.02/
> Which makes me wonder if atomull needs renaming - does the "m" mean
> memory? atojulong would seem more appropriate regardless.
I renamed to atoju
Hi Yasumasa,
On 19/04/2016 12:06 AM, Yasumasa Suenaga wrote:
> PING:
>
> I've sent review request for JDK-8153073.
> Could you review it?
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
>
> If this patch is merged, user can set logfile size with k/m/g.
Your webrev seems out
PING:
I've sent review request for JDK-8153073.
Could you review it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
If this patch is merged, user can set logfile size with k/m/g.
Please review it.
Thanks,
Yasumasa
On 2016/04/11 18:28, Yasumasa Suenaga wrote:
> PING: Could
PING: Could you review it?
We need more reviewer.
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
Thanks,
Yasumasa
On 2016/03/31 22:33, Yasumasa Suenaga wrote:
> CC'ed to serviceability-dev.
>
> Could you review it?
>
>> http://cr.openjdk.java.net/~ysuenaga/JDK-815307
CC'ed to serviceability-dev.
Could you review it?
>http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
Thanks,
Yasumasa
On 2016/03/31 18:24, Yasumasa Suenaga wrote:
> Hi Marcus,
>
>> You're missing an include of arguments.hpp in logFileOutput.cpp.
>
> arguments.hpp is included
28 matches
Mail list logo
