Quantcast

[10] RFR(XS): 8180473: Use proper deallocation for FileBuff::_bigbuf

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[10] RFR(XS): 8180473: Use proper deallocation for FileBuff::_bigbuf

Zoltán Majó
Hi,


please review the following fix for 8180473.
https://bugs.openjdk.java.net/browse/JDK-8180473
http://cr.openjdk.java.net/~zmajo/8180473/webrev.00/

It is a good coding practice to properly deallocate dynamically
allocated resources. In particular, dynamically allocated memory for
arrays using the array-specific new[] operator is supposed to be
deallocated using the array-specific delete[] operator. The deallocation
of FileBuff::_bigbuf does not follow this recommendation and should
therefore be changed. Tested with JPRT.

Thank you!

Best regards,


Zoltan

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR(XS): 8180473: Use proper deallocation for FileBuff::_bigbuf

Vladimir Kozlov
Good.

Thanks,
Vladimir

On 5/17/17 3:52 AM, Zoltán Majó wrote:

> Hi,
>
>
> please review the following fix for 8180473.
> https://bugs.openjdk.java.net/browse/JDK-8180473
> http://cr.openjdk.java.net/~zmajo/8180473/webrev.00/
>
> It is a good coding practice to properly deallocate dynamically allocated resources. In particular, dynamically allocated memory for arrays using the array-specific new[] operator is supposed to be
> deallocated using the array-specific delete[] operator. The deallocation of FileBuff::_bigbuf does not follow this recommendation and should therefore be changed. Tested with JPRT.
>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR(XS): 8180473: Use proper deallocation for FileBuff::_bigbuf

Zoltán Majó
Thank you, Vladimir, for the review!

Best regards,


Zoltan


On 05/18/2017 01:46 AM, Vladimir Kozlov wrote:

> Good.
>
> Thanks,
> Vladimir
>
> On 5/17/17 3:52 AM, Zoltán Majó wrote:
>> Hi,
>>
>>
>> please review the following fix for 8180473.
>> https://bugs.openjdk.java.net/browse/JDK-8180473
>> http://cr.openjdk.java.net/~zmajo/8180473/webrev.00/
>>
>> It is a good coding practice to properly deallocate dynamically
>> allocated resources. In particular, dynamically allocated memory for
>> arrays using the array-specific new[] operator is supposed to be
>> deallocated using the array-specific delete[] operator. The
>> deallocation of FileBuff::_bigbuf does not follow this recommendation
>> and should therefore be changed. Tested with JPRT.
>>
>> Thank you!
>>
>> Best regards,
>>
>>
>> Zoltan
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR(XS): 8180473: Use proper deallocation for FileBuff::_bigbuf

Thomas Stüfe-2
In reply to this post by Zoltán Majó
Hi Zoltan,

looks good. Old code may have worked but relied on undefined behaviour.

Kind Regards, Thomas

On Wed, May 17, 2017 at 12:52 PM, Zoltán Majó <[hidden email]> wrote:
Hi,


please review the following fix for 8180473.
https://bugs.openjdk.java.net/browse/JDK-8180473
http://cr.openjdk.java.net/~zmajo/8180473/webrev.00/

It is a good coding practice to properly deallocate dynamically allocated resources. In particular, dynamically allocated memory for arrays using the array-specific new[] operator is supposed to be deallocated using the array-specific delete[] operator. The deallocation of FileBuff::_bigbuf does not follow this recommendation and should therefore be changed. Tested with JPRT.

Thank you!

Best regards,


Zoltan


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR(XS): 8180473: Use proper deallocation for FileBuff::_bigbuf

Zoltán Majó
Hi Thomas,


On 05/18/2017 08:47 PM, Thomas Stüfe wrote:
> Hi Zoltan,
>
> looks good. Old code may have worked but relied on undefined behaviour.

thank you for the review.

Unfortunately, the fix was already in the repo when your mail arrived,
so I am not able to mention you as a reviewer. Sorry for that.

Best regards,


Zoltan

>
> Kind Regards, Thomas
>
> On Wed, May 17, 2017 at 12:52 PM, Zoltán Majó <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi,
>
>
>     please review the following fix for 8180473.
>     https://bugs.openjdk.java.net/browse/JDK-8180473
>     <https://bugs.openjdk.java.net/browse/JDK-8180473>
>     http://cr.openjdk.java.net/~zmajo/8180473/webrev.00/
>     <http://cr.openjdk.java.net/%7Ezmajo/8180473/webrev.00/>
>
>     It is a good coding practice to properly deallocate dynamically
>     allocated resources. In particular, dynamically allocated memory
>     for arrays using the array-specific new[] operator is supposed to
>     be deallocated using the array-specific delete[] operator. The
>     deallocation of FileBuff::_bigbuf does not follow this
>     recommendation and should therefore be changed. Tested with JPRT.
>
>     Thank you!
>
>     Best regards,
>
>
>     Zoltan
>
>

Loading...