This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]