Re: [PATCH 2/3] gdb: make macro_expand_next return a gdb::unique_xmalloc_ptr<char>

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2020-06-30 4:53 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> I removed the ATTRIBUTE_UNUSED_RESULT on macro_buffer::release, I don't
> Simon> really understand why it's there.  I don't see how this method could be
> Simon> called without using the result, that would be an obvious memory leak.
> Simon> The commit that introduced it (4e4a8b932b7 "Add ATTRIBUTE_UNUSED_RESULT
> Simon> to macro_buffer") doesn't give any details.
> 
> Sorry about the lack of details!
> 
> Simon> -  /* Release the text of the buffer to the caller, which is now
> Simon> -     responsible for freeing it.  */
> Simon> -  ATTRIBUTE_UNUSED_RESULT char *release ()
> Simon> +  /* Release the text of the buffer to the caller.  */
> Simon> +  gdb::unique_xmalloc_ptr<char> release ()
> 
> In the old code, the caller had to remember to free the resulting char*.
> ATTRIBUTE_UNUSED_RESULT meant that one couldn't write "x.release();"
> without at least doing something with the result.
> 
> There was at least one case of this happening, though I didn't read
> through the thread to find out exactly where it was:
> 
> https://sourceware.org/pipermail/gdb-patches/2019-February/155835.html
> 
> Using a unique pointer eliminates this common source of bugs, so I think
> removing the attribute makes sense.
> 
> This patch looks fine to me.
> 
> Tom
> 

Ok thanks, that makes sense!

Simon