This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH,Hurd] Fix deallocation after proc_getprocinfo call
- From: Joel Brobecker <brobecker at adacore dot com>
- To: bug-hurd at gnu dot org, thomas at codesourcery dot com, gdb-patches at sourceware dot org
- Date: Sun, 23 Nov 2014 11:44:52 +0400
- Subject: Re: [PATCH,Hurd] Fix deallocation after proc_getprocinfo call
- Authentication-results: sourceware.org; auth=none
- References: <20141102152537 dot GG2991 at type dot youpi dot perso dot aquilenet dot fr>
Hello Samuel,
On Sun, Nov 02, 2014 at 04:25:37PM +0100, Samuel Thibault wrote:
> 2014-10-02 Samuel Thibault <samuel.thibault@ens-lyon.org>
>
> * gdb/gnu-nat.c (inf_validate_procinfo): Multiply the number of
> elements pi_len by the size of the elements before calling
> vm_deallocate.
> (inf_validate_task_sc): Likewise, and properly deallocate the
> noise array.
Again, sorry about the late review...
I only have a few minor comments, almost trivial in nature. In
the ChangeLog entry above, watch out that the last 2 lines are
indented using spaces intead of tabs.
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index d17a750..c571190 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -804,7 +804,7 @@ inf_validate_procinfo (struct inf *inf)
> inf->nomsg = !!(pi->state & PI_NOMSG);
> if (inf->nomsg)
> inf->traced = !!(pi->state & PI_TRACED);
> - vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len);
> + vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len * sizeof (*(procinfo_t) 0));
The line is too long (soft limit is 74 characters, hard limit is 80).
Suggest using "sizeof (struct procinfo)", which I think is better
than dereferencing a NULL pointer. This is based on guessing that
type procinfo_t is a pointer to struct procinfo, as suggested by
the code in inf_validate_procinfo.
> if (noise_len > 0)
> vm_deallocate (mach_task_self (), (vm_address_t) noise, noise_len);
> }
> @@ -844,9 +844,9 @@ inf_validate_task_sc (struct inf *inf)
>
> suspend_count = pi->taskinfo.suspend_count;
>
> - vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len);
> + vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len * sizeof (*(procinfo_t) 0));
Same as above.
> if (noise_len > 0)
> - vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len);
> + vm_deallocate (mach_task_self (), (vm_address_t) noise, noise_len);
>
> if (inf->task->cur_sc < suspend_count)
> {
Thank you,
--
Joel