mbox series

[0/4] Modernize and fix leaks around internal_function

Message ID 20250310151211.2708331-1-simon.marchi@polymtl.ca
Headers
Series Modernize and fix leaks around internal_function |

Message

Simon Marchi March 10, 2025, 3:10 p.m. UTC 
  From: Simon Marchi <simon.marchi@polymtl.ca>

This series fixes a small leak around internalvar_function, and does
some mild C++-ification.

When running:

  $ ./gdb -q -nx --data-directory=data-directory -batch

I go from:

  SUMMARY: AddressSanitizer: 150845 byte(s) leaked in 1390 allocation(s).

to:

  SUMMARY: AddressSanitizer: 149282 byte(s) leaked in 1340 allocation(s).

Woohoo, almost there!

Simon Marchi (4):
  gdb: C++-ify internal_function
  gdb: clear internalvar on destruction
  gdb: handle INTERNALVAR_FUNCTION in clear_internalvar
  gdb: change set_internalvar_function to take a unique pointer

 gdb/value.c | 54 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 21 deletions(-)


base-commit: 48691f6650bbae30d702faa2fddb666763ee4995

Comments

Guinevere Larsen March 19, 2025, 6:58 p.m. UTC | #1 
On 3/10/25 12:10 PM, simon.marchi@polymtl.ca wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
>
> This series fixes a small leak around internalvar_function, and does
> some mild C++-ification.
>
> When running:
>
>    $ ./gdb -q -nx --data-directory=data-directory -batch
>
> I go from:
>
>    SUMMARY: AddressSanitizer: 150845 byte(s) leaked in 1390 allocation(s).
>
> to:
>
>    SUMMARY: AddressSanitizer: 149282 byte(s) leaked in 1340 allocation(s).
>
> Woohoo, almost there!
>
> Simon Marchi (4):
>    gdb: C++-ify internal_function
>    gdb: clear internalvar on destruction
>    gdb: handle INTERNALVAR_FUNCTION in clear_internalvar
>    gdb: change set_internalvar_function to take a unique pointer

I think this is a nice fix, and adds no regression! Reviewed-By: 
Guinevere Larsen <guinevere@redhat.com>

I do wonder if it would make sense to make internalvar hold a unique 
pointer as well, but I haven't tested and that's just suggestions for 
future improvement
Simon Marchi March 19, 2025, 7:17 p.m. UTC | #2 
On 2025-03-19 14:58, Guinevere Larsen wrote:
> On 3/10/25 12:10 PM, simon.marchi@polymtl.ca wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>> This series fixes a small leak around internalvar_function, and does
>> some mild C++-ification.
>>
>> When running:
>>
>>    $ ./gdb -q -nx --data-directory=data-directory -batch
>>
>> I go from:
>>
>>    SUMMARY: AddressSanitizer: 150845 byte(s) leaked in 1390 allocation(s).
>>
>> to:
>>
>>    SUMMARY: AddressSanitizer: 149282 byte(s) leaked in 1340 allocation(s).
>>
>> Woohoo, almost there!
>>
>> Simon Marchi (4):
>>    gdb: C++-ify internal_function
>>    gdb: clear internalvar on destruction
>>    gdb: handle INTERNALVAR_FUNCTION in clear_internalvar
>>    gdb: change set_internalvar_function to take a unique pointer
> 
> I think this is a nice fix, and adds no regression! Reviewed-By: Guinevere Larsen <guinevere@redhat.com>

Thanks, I will push it.

Btw, if you don't put your tag on a line of its own, it doesn't get
picked up by b4.

> I do wonder if it would make sense to make internalvar hold a unique
> pointer as well, but I haven't tested and that's just suggestions for
> future improvement

It would, but we can't put a unique pointer inside of a union, so some
non-trivial refactoring needs to happen.  One option would be to use
std::variant.

Simon