[PATCH 3/4] Support 'info proc' for native FreeBSD processes.

Simon Marchi simark@simark.ca
Wed Jan 3 19:13:00 GMT 2018


Hi John,

>> > +#ifdef HAVE_KINFO_GETVMMAP
>> > +  if (do_mappings)
>> > +    {
>> > +      int nvment;
>> > +      std::unique_ptr<struct kinfo_vmentry, free_deleter<struct kinfo_vmentry>>
>> 
>> Is there a reason to have and use free_deleter rather than 
>> gdb::unique_xmalloc_ptr?
>> 
>> > +	vmentl (kinfo_getvmmap (pid, &nvment));
> 
> This function (kinfo_getvmmap) which is defined in the libutil library
> included in
> FreeBSD's base system calls malloc() internally, so the memory returned 
> must be
> freed with free() rather than xfree().  This deleter is already used 
> earlier in
> fbsd_find_memory_regions() for another call to kinfo_getvmmap() for
> the same reason.

But isn't xfree just a wrapper around free?

>> > +
>> > +      if (vmentl)
>> 
>> vmentl != NULL
>> 
>> There are a few other instances of if (ptr) that should be changed to 
>> if (ptr != NULL).
> 
> Ok.  The style in GDB doesn't seem consistent in this regard (I
> largely based this
> code on the 'info proc' implementation in linux-tdep.c which doesn't 
> explicitly
> compare against NULL/nullptr (though I prefer the explicit comparison 
> myself)).
> Also, I feel like we should use nullptr rather than NULL when working
> with "smart"
> pointer types like unique_ptr<> at least?

You are right, there is code that comes from an era where the coding 
style wasn't as strictly enforced, it seems.  I don't think we should go 
fix coding style issues just for fun, but when we modify or copy 
existing code, we should make it respect the style.

Thanks,

Simon



More information about the Binutils mailing list