[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH,Hurd] Fix deallocation after proc_getprocinfo call
From: |
Joel Brobecker |
Subject: |
Re: [PATCH,Hurd] Fix deallocation after proc_getprocinfo call |
Date: |
Sun, 23 Nov 2014 11:44:52 +0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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