gdb: Remove unused extra_lines variable

Message ID 3090ae2157c8eae596b1bf5989c1853b865fdacf.1664093762.git.research_trasio@irq.a4lg.com
State Committed
Commit f3700471469ac0f395961e92892bc5d6e17ca177
Headers
Series gdb: Remove unused extra_lines variable |

Commit Message

Tsukasa OI Sept. 25, 2022, 8:17 a.m. UTC
  Clang generates a warning if there is a variable that is set but not used
otherwise ("-Wunused-but-set-variable").  On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).

The only extra_lines use in arrange_linetable function is removed on the
commit 558802e4d1c5dcbd0df7d2c6ef62a6deac247a2f
("gdb: change subfile::line_vector to an std::vector").  So, this variable
should be removed to prevent a build failure.

gdb/ChangeLog:

	* xcoffread.c (arrange_linetable): Remove unused extra_lines.
---
 gdb/xcoffread.c | 8 --------
 1 file changed, 8 deletions(-)


base-commit: 58d69206b8173b9d027a6c65f56cdaf045ae6e64
  

Comments

Bruno Larsen Sept. 26, 2022, 9 a.m. UTC | #1
On 25/09/2022 10:17, Tsukasa OI via Gdb-patches wrote:
> Clang generates a warning if there is a variable that is set but not used
> otherwise ("-Wunused-but-set-variable").  On the default configuration, it
> causes a build failure (unless "--disable-werror" is specified).
>
> The only extra_lines use in arrange_linetable function is removed on the
> commit 558802e4d1c5dcbd0df7d2c6ef62a6deac247a2f
> ("gdb: change subfile::line_vector to an std::vector").  So, this variable
> should be removed to prevent a build failure.

Hi Tsukasa!

This patch looks pretty safe. The point at which extra_lines would be 
used has the exact same check used to set it, instead of using the 
variable, so I think there is no need for it after all.

That said, I can't approve the patch for pushing, so I hope a maintainer 
looks at this soon.

>
> gdb/ChangeLog:
>
> 	* xcoffread.c (arrange_linetable): Remove unused extra_lines.
side note, GDB doesn't require changelogs anymore, but there are no 
rules against it, so you can leave it here if you'd like.

Cheers,
Bruno

> ---
>   gdb/xcoffread.c | 8 --------
>   1 file changed, 8 deletions(-)
>
> diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
> index b7d65771115..aa88cbc724d 100644
> --- a/gdb/xcoffread.c
> +++ b/gdb/xcoffread.c
> @@ -419,8 +419,6 @@ add_stab_to_list (char *stabname, struct pending_stabs **stabvector)
>   static void
>   arrange_linetable (std::vector<linetable_entry> &old_linetable)
>   {
> -  int extra_lines = 0;
> -
>     std::vector<linetable_entry> fentries;
>   
>     for (int ii = 0; ii < old_linetable.size (); ++ii)
> @@ -436,12 +434,6 @@ arrange_linetable (std::vector<linetable_entry> &old_linetable)
>   	  e.line = ii;
>   	  e.is_stmt = 1;
>   	  e.pc = old_linetable[ii].pc;
> -
> -	  /* If the function was compiled with XLC, we may have to add an
> -	     extra line entry later.  Reserve space for that.  */
> -	  if (ii + 1 < old_linetable.size ()
> -	      && old_linetable[ii].pc != old_linetable[ii + 1].pc)
> -	    extra_lines++;
>   	}
>       }
>   
>
> base-commit: 58d69206b8173b9d027a6c65f56cdaf045ae6e64
  
Tom Tromey Sept. 29, 2022, 5:22 p.m. UTC | #2
>>>>> Tsukasa OI via Gdb-patches <gdb-patches@sourceware.org> writes:

> Clang generates a warning if there is a variable that is set but not used
> otherwise ("-Wunused-but-set-variable").  On the default configuration, it
> causes a build failure (unless "--disable-werror" is specified).

> The only extra_lines use in arrange_linetable function is removed on the
> commit 558802e4d1c5dcbd0df7d2c6ef62a6deac247a2f
> ("gdb: change subfile::line_vector to an std::vector").  So, this variable
> should be removed to prevent a build failure.

Thanks, this is ok.

Tom
  
Tsukasa OI Sept. 30, 2022, 3:50 p.m. UTC | #3
On 2022/09/26 18:00, Bruno Larsen wrote:
> On 25/09/2022 10:17, Tsukasa OI via Gdb-patches wrote:
>> Clang generates a warning if there is a variable that is set but not used
>> otherwise ("-Wunused-but-set-variable").  On the default
>> configuration, it
>> causes a build failure (unless "--disable-werror" is specified).
>>
>> The only extra_lines use in arrange_linetable function is removed on the
>> commit 558802e4d1c5dcbd0df7d2c6ef62a6deac247a2f
>> ("gdb: change subfile::line_vector to an std::vector").  So, this
>> variable
>> should be removed to prevent a build failure.
> 
> Hi Tsukasa!
> 
> This patch looks pretty safe. The point at which extra_lines would be
> used has the exact same check used to set it, instead of using the
> variable, so I think there is no need for it after all.
> 
> That said, I can't approve the patch for pushing, so I hope a maintainer
> looks at this soon.
> 
>>
>> gdb/ChangeLog:
>>
>>     * xcoffread.c (arrange_linetable): Remove unused extra_lines.
> side note, GDB doesn't require changelogs anymore, but there are no
> rules against it, so you can leave it here if you'd like.

Thanks for letting me know!

A maintainer (Tom) approved the patch and I removed the ChangeLog in the
commit message (as an obvious change) and then pushed.  The original
intent of ChangeLog was to keep formatting as common as Binutils but it
required extra work (contrib/mklog.py does not work properly on GDB
files) and I think I looked pretty old version of contribution guide or
something when I decided to do so.

Thanks,
Tsukasa

> 
> Cheers,
> Bruno
> 
>> ---
>>   gdb/xcoffread.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
>> index b7d65771115..aa88cbc724d 100644
>> --- a/gdb/xcoffread.c
>> +++ b/gdb/xcoffread.c
>> @@ -419,8 +419,6 @@ add_stab_to_list (char *stabname, struct
>> pending_stabs **stabvector)
>>   static void
>>   arrange_linetable (std::vector<linetable_entry> &old_linetable)
>>   {
>> -  int extra_lines = 0;
>> -
>>     std::vector<linetable_entry> fentries;
>>       for (int ii = 0; ii < old_linetable.size (); ++ii)
>> @@ -436,12 +434,6 @@ arrange_linetable (std::vector<linetable_entry>
>> &old_linetable)
>>         e.line = ii;
>>         e.is_stmt = 1;
>>         e.pc = old_linetable[ii].pc;
>> -
>> -      /* If the function was compiled with XLC, we may have to add an
>> -         extra line entry later.  Reserve space for that.  */
>> -      if (ii + 1 < old_linetable.size ()
>> -          && old_linetable[ii].pc != old_linetable[ii + 1].pc)
>> -        extra_lines++;
>>       }
>>       }
>>  
>> base-commit: 58d69206b8173b9d027a6c65f56cdaf045ae6e64
>
  

Patch

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index b7d65771115..aa88cbc724d 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -419,8 +419,6 @@  add_stab_to_list (char *stabname, struct pending_stabs **stabvector)
 static void
 arrange_linetable (std::vector<linetable_entry> &old_linetable)
 {
-  int extra_lines = 0;
-
   std::vector<linetable_entry> fentries;
 
   for (int ii = 0; ii < old_linetable.size (); ++ii)
@@ -436,12 +434,6 @@  arrange_linetable (std::vector<linetable_entry> &old_linetable)
 	  e.line = ii;
 	  e.is_stmt = 1;
 	  e.pc = old_linetable[ii].pc;
-
-	  /* If the function was compiled with XLC, we may have to add an
-	     extra line entry later.  Reserve space for that.  */
-	  if (ii + 1 < old_linetable.size ()
-	      && old_linetable[ii].pc != old_linetable[ii + 1].pc)
-	    extra_lines++;
 	}
     }