[PATCH v2] Hurd: remove VLA usage.

[PATCH v2] Hurd: remove VLA usage.

[Flavio Cruz]

Compilation will fail with -Werror=vla, which seems to be the default.

Note that we don't need to allocate num_threads + 1 since the matching
algorithm works only on the num_threads as returned by task_threads.
---
 gdb/gnu-nat.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index a8a4da1c..be6fa879 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -1016,15 +1016,13 @@ gnu_nat_target::inf_validate_procs (struct inf *inf)
   {
     /* Make things normally linear.  */
     mach_msg_type_number_t search_start = 0;
-    /* Which thread in PROCS corresponds to each task thread, & the task.  */
-    struct proc *matched[num_threads + 1];
+    /* Which thread in PROCS corresponds to each task thread.  */
+    std::vector<struct proc *> matched (num_threads, nullptr);
     /* The last thread in INF->threads, so we can add to the end.  */
     struct proc *last = 0;
     /* The current thread we're considering.  */
     struct proc *thread = inf->threads;
 
-    memset (matched, 0, sizeof (matched));
-
     while (thread)
       {
 	mach_msg_type_number_t left;
-- 
2.45.2

Re: [PATCH v2] Hurd: remove VLA usage.

[Simon Marchi]

On 2024-12-19 00:28, Flavio Cruz wrote:
> Compilation will fail with -Werror=vla, which seems to be the default.
> 
> Note that we don't need to allocate num_threads + 1 since the matching
> algorithm works only on the num_threads as returned by task_threads.

I don't fully understand the algorithm, but I see that `matched` only
gets indexed using `i`, and `i` is always less than `num_threads`, so
this seems fine to me.

> ---
>  gdb/gnu-nat.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index a8a4da1c..be6fa879 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -1016,15 +1016,13 @@ gnu_nat_target::inf_validate_procs (struct inf *inf)
>    {
>      /* Make things normally linear.  */
>      mach_msg_type_number_t search_start = 0;
> -    /* Which thread in PROCS corresponds to each task thread, & the task.  */
> -    struct proc *matched[num_threads + 1];
> +    /* Which thread in PROCS corresponds to each task thread.  */
> +    std::vector<struct proc *> matched (num_threads, nullptr);

It's not necessary to specify nullptr here, because the pointers will be
default initialized, which means nullptr for pointers.

LGTM with that fixed.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Do you have push access?

Simon

[PATCH v2] Hurd: remove VLA usage.

[Flavio Cruz]

Compilation will fail with -Werror=vla, which seems to be the default.

Note that we don't need to allocate num_threads + 1 since the matching
algorithm works only on the num_threads as returned by task_threads.
---
 gdb/gnu-nat.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index a8a4da1c..ab157094 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -1016,15 +1016,13 @@ gnu_nat_target::inf_validate_procs (struct inf *inf)
   {
     /* Make things normally linear.  */
     mach_msg_type_number_t search_start = 0;
-    /* Which thread in PROCS corresponds to each task thread, & the task.  */
-    struct proc *matched[num_threads + 1];
+    /* Which thread in PROCS corresponds to each task thread.  */
+    std::vector<struct proc *> matched (num_threads);
     /* The last thread in INF->threads, so we can add to the end.  */
     struct proc *last = 0;
     /* The current thread we're considering.  */
     struct proc *thread = inf->threads;
 
-    memset (matched, 0, sizeof (matched));
-
     while (thread)
       {
 	mach_msg_type_number_t left;
-- 
2.45.2

Re: [PATCH v2] Hurd: remove VLA usage.

[Simon Marchi]

On 2024-12-22 00:34, Flavio Cruz wrote:
> Compilation will fail with -Werror=vla, which seems to be the default.
> 
> Note that we don't need to allocate num_threads + 1 since the matching
> algorithm works only on the num_threads as returned by task_threads.
> ---
>  gdb/gnu-nat.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index a8a4da1c..ab157094 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -1016,15 +1016,13 @@ gnu_nat_target::inf_validate_procs (struct inf *inf)
>    {
>      /* Make things normally linear.  */
>      mach_msg_type_number_t search_start = 0;
> -    /* Which thread in PROCS corresponds to each task thread, & the task.  */
> -    struct proc *matched[num_threads + 1];
> +    /* Which thread in PROCS corresponds to each task thread.  */
> +    std::vector<struct proc *> matched (num_threads);
>      /* The last thread in INF->threads, so we can add to the end.  */
>      struct proc *last = 0;
>      /* The current thread we're considering.  */
>      struct proc *thread = inf->threads;
>  
> -    memset (matched, 0, sizeof (matched));
> -
>      while (thread)
>        {
>  	mach_msg_type_number_t left;

Thanks, I pushed that for you.  I made a minor modification, adding
newlines before the comments between the declarations.

Simon

Re: [PATCH v2] Hurd: remove VLA usage.

[Flávio Cruz]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

On Thu, Dec 19, 2024 at 10:05 AM Simon Marchi <simark@simark.ca> wrote:

>
>
> On 2024-12-19 00:28, Flavio Cruz wrote:
> > Compilation will fail with -Werror=vla, which seems to be the default.
> >
> > Note that we don't need to allocate num_threads + 1 since the matching
> > algorithm works only on the num_threads as returned by task_threads.
>
> I don't fully understand the algorithm, but I see that `matched` only
> gets indexed using `i`, and `i` is always less than `num_threads`, so
> this seems fine to me.
>
> > ---
> >  gdb/gnu-nat.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> > index a8a4da1c..be6fa879 100644
> > --- a/gdb/gnu-nat.c
> > +++ b/gdb/gnu-nat.c
> > @@ -1016,15 +1016,13 @@ gnu_nat_target::inf_validate_procs (struct inf
> *inf)
> >    {
> >      /* Make things normally linear.  */
> >      mach_msg_type_number_t search_start = 0;
> > -    /* Which thread in PROCS corresponds to each task thread, & the
> task.  */
> > -    struct proc *matched[num_threads + 1];
> > +    /* Which thread in PROCS corresponds to each task thread.  */
> > +    std::vector<struct proc *> matched (num_threads, nullptr);
>
> It's not necessary to specify nullptr here, because the pointers will be
> default initialized, which means nullptr for pointers.
>

Good point, I've sent a new patch.


>
> LGTM with that fixed.
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>
> Do you have push access?
>

No, I don't. Thanks for the review.


>
> Simon
>

[-- Attachment #2: Type: text/html, Size: 2515 bytes --]