mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joerg Schad" <jo...@mesosphere.io>
Subject Re: Review Request 31489: Fix use of buffers in OS to remove leaks.
Date Thu, 26 Feb 2015 20:54:22 GMT


> On Feb. 26, 2015, 6:14 p.m., Ben Mahler wrote:
> > This is pretty inconsistent with the rest of our code. Why can't you add the missing
deletes?
> 
> Dominic Hamon wrote:
>     we can. but why not start getting rid of the archaic practice of using raw pointers?
> 
> Timothy Chen wrote:
>     I agree having less prone to errors is more desired.
> 
> Ben Mahler wrote:
>     From the title, it looks like this would have changed all buffers in os.hpp, rather
than introducing inconsistency in the process of fixing the two leaks (detected by coverity).
>     
>     More importantly, is '`vector`' an intuitive way to capture a fixed size buffer on
the heap? I'm all for improving this, but we need to think more carefully about how to represent
buffers, do we want to introduce some kind of '`Buffer`' abstraction if benefical to readability
(e.g. implicitly castable to the pointer type, etc), or consider `Owned<>`, `unique_ptr`,
`scoped_ptr`, etc? Let's do these kind of changes independently.

I must say that I don't like the use of raw pointers here (and agree with Dominic, Timothy
and Alexander on this), but Ben is correct is pointing out that these are two independent
changes: removing the memleak and introducing a better(TBD) datastructure.
As I actually had a patch just introducing the two delete[] ready this morning (but then Dominic
was faster :-) ) I just that one to RB here: https://reviews.apache.org/r/31494/ .


- Joerg


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31489/#review74305
-----------------------------------------------------------


On Feb. 26, 2015, 6:14 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31489/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2015, 6:14 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Till Toenshoff.
> 
> 
> Bugs: MESOS-2412
>     https://issues.apache.org/jira/browse/MESOS-2412
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fix use of buffers in OS to remove leaks.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 8a4fda97ee29c185471a69f60803efc193cbe922

> 
> Diff: https://reviews.apache.org/r/31489/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message