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

[Simon Marchi <simark@simark.ca>]

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