diff mbox

[09/10] Class-ify lm_info_darwin

Message ID 20170426225139.313-9-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi April 26, 2017, 10:51 p.m. UTC 
  This patch makes lm_info_darwin a "real" class.  It initializes the
field and replaces XCNEW/xfree with new/delete.

I believe the cleanup in darwin_current_sos can be removed, I don't see
anything that can throw after the allocation and the call to
discard_cleanups.

gdb/ChangeLog:

	* solib-darwin.c (struct lm_info_darwin): Initialize field.
	(darwin_current_sos): Allocate lm_info_darwin with new, remove
	cleanup.
	(darwin_free_so): Free lm_info_darwin with delete.
---
 gdb/solib-darwin.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Pedro Alves April 28, 2017, 4 p.m. UTC | #1 
On 04/26/2017 11:51 PM, Simon Marchi wrote:
> This patch makes lm_info_darwin a "real" class.  It initializes the
> field and replaces XCNEW/xfree with new/delete.
> 
> I believe the cleanup in darwin_current_sos can be removed, I don't see
> anything that can throw after the allocation and the call to
> discard_cleanups.

Agreed.

LGTM but ...

> --- a/gdb/solib-darwin.c
> +++ b/gdb/solib-darwin.c
> @@ -41,6 +41,8 @@
>  #include "mach-o.h"
>  #include "mach-o/external.h"
>  
> +#include <memory>

... this doesn't look necessary?  (it was not in the ChangeLog.)

Thanks,
Pedro Alves
Simon Marchi April 28, 2017, 9:13 p.m. UTC | #2 
On 2017-04-28 12:00, Pedro Alves wrote:
> On 04/26/2017 11:51 PM, Simon Marchi wrote:
>> This patch makes lm_info_darwin a "real" class.  It initializes the
>> field and replaces XCNEW/xfree with new/delete.
>> 
>> I believe the cleanup in darwin_current_sos can be removed, I don't 
>> see
>> anything that can throw after the allocation and the call to
>> discard_cleanups.
> 
> Agreed.
> 
> LGTM but ...
> 
>> --- a/gdb/solib-darwin.c
>> +++ b/gdb/solib-darwin.c
>> @@ -41,6 +41,8 @@
>>  #include "mach-o.h"
>>  #include "mach-o/external.h"
>> 
>> +#include <memory>
> 
> ... this doesn't look necessary?  (it was not in the ChangeLog.)

Ah, the comment in the commit log and this include is a leftover.  While 
doing the preparatory patch for darwin (01/10), I had put an xfree 
cleanup for the lm_info_darwin structure, but I later removed it.

Should we expect that the "new" operator can throw if memory allocation 
fails?  In that case we can't get rid of the cleanup for newobj.

Simon
Pedro Alves May 2, 2017, 2:42 p.m. UTC | #3 
On 04/28/2017 10:13 PM, Simon Marchi wrote:
> On 2017-04-28 12:00, Pedro Alves wrote:
>> On 04/26/2017 11:51 PM, Simon Marchi wrote:
>>> This patch makes lm_info_darwin a "real" class.  It initializes the
>>> field and replaces XCNEW/xfree with new/delete.
>>>
>>> I believe the cleanup in darwin_current_sos can be removed, I don't see
>>> anything that can throw after the allocation and the call to
>>> discard_cleanups.
>>
>> Agreed.
>>
>> LGTM but ...
>>
>>> --- a/gdb/solib-darwin.c
>>> +++ b/gdb/solib-darwin.c
>>> @@ -41,6 +41,8 @@
>>>  #include "mach-o.h"
>>>  #include "mach-o/external.h"
>>>
>>> +#include <memory>
>>
>> ... this doesn't look necessary?  (it was not in the ChangeLog.)
> 
> Ah, the comment in the commit log and this include is a leftover.  While
> doing the preparatory patch for darwin (01/10), I had put an xfree
> cleanup for the lm_info_darwin structure, but I later removed it.
> 
> Should we expect that the "new" operator can throw if memory allocation
> fails?  In that case we can't get rid of the cleanup for newobj.

Yeah, pedantically new can throw here.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 03211cfb92..0e20dc4fc5 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -41,6 +41,8 @@ 
 #include "mach-o.h"
 #include "mach-o/external.h"
 
+#include <memory>
+
 struct gdb_dyld_image_info
 {
   /* Base address (which corresponds to the Mach-O header).  */
@@ -156,7 +158,7 @@  darwin_load_image_infos (struct darwin_info *info)
 struct lm_info_darwin : public lm_info_base
 {
   /* The target location of lm.  */
-  CORE_ADDR lm_addr;
+  CORE_ADDR lm_addr = 0;
 };
 
 /* Lookup the value for a specific symbol.  */
@@ -296,7 +298,7 @@  darwin_current_sos (void)
       newobj = XCNEW (struct so_list);
       old_chain = make_cleanup (xfree, newobj);
 
-      lm_info_darwin *li = XCNEW (lm_info_darwin);
+      lm_info_darwin *li = new lm_info_darwin;
       newobj->lm_info = li;
 
       strncpy (newobj->so_name, file_path, SO_NAME_MAX_PATH_SIZE - 1);
@@ -578,7 +580,9 @@  darwin_clear_solib (void)
 static void
 darwin_free_so (struct so_list *so)
 {
-  xfree (so->lm_info);
+  lm_info_darwin *li = (lm_info_darwin *) so->lm_info;
+
+  delete li;
 }
 
 /* The section table is built from bfd sections using bfd VMAs.