diff mbox

[v2,07/22] Remove last cleanups from solib-svr4.c

Message ID 20190227201849.32210-8-tom@tromey.com
State New
Headers show

Commit Message

Tom Tromey Feb. 27, 2019, 8:18 p.m. UTC
This removes the last cleanups from solib-svr4.c, replacing them with
uses of make_scope_exit.

gdb/ChangeLog
2019-02-27  Tom Tromey  <tom@tromey.com>

	* solib-svr4.c (svr4_parse_libraries, svr4_current_sos_direct):
	Use make_scope_exit.
---
 gdb/ChangeLog    |  5 +++++
 gdb/solib-svr4.c | 17 ++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

Comments

Pedro Alves March 6, 2019, 7:32 p.m. UTC | #1
On 02/27/2019 08:18 PM, Tom Tromey wrote:
> This removes the last cleanups from solib-svr4.c, replacing them with
> uses of make_scope_exit.
> 
> gdb/ChangeLog
> 2019-02-27  Tom Tromey  <tom@tromey.com>
> 
> 	* solib-svr4.c (svr4_parse_libraries, svr4_current_sos_direct):
> 	Use make_scope_exit.
> ---
>  gdb/ChangeLog    |  5 +++++
>  gdb/solib-svr4.c | 17 ++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 2b370ef96d0..2301cf94c2f 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -1198,8 +1198,10 @@ static const struct gdb_xml_element svr4_library_list_elements[] =
>  static int
>  svr4_parse_libraries (const char *document, struct svr4_library_list *list)
>  {
> -  struct cleanup *back_to = make_cleanup (svr4_free_library_list,
> -					  &list->head);
> +  auto cleanup = make_scope_exit ([&] ()
> +				  {
> +				    svr4_free_library_list (&list->head);
> +				  });
>  

When passing lambdas as last argument to a function, I think indenting
like this reads clearer:

  auto cleanup = make_scope_exit ([&] ()
    {
      svr4_free_library_list (&list->head);
    });

Makes it read more like an if/for scope block, and of course,
avoids aligning things too much to the right side.  Examples of
this can be found throughout gdbserver, for example:

	  for_each_thread ([] (thread_info *thread)
	    {
	      thread->status_pending_p = 0;
	    });

and, also, that's how you'd indent if you used SCOPE_EXIT instead:

  SCOPE_EXIT
    {
      svr4_free_library_list (&list->head);
    }

LLVM has a written rule for this:

 https://llvm.org/docs/CodingStandards.html#format-lambdas-like-blocks-of-code

If others agree, I'd vote for having a similar rule of our own.

Otherwise, LGTM.

Thanks,
Pedro Alves
John Baldwin March 6, 2019, 7:39 p.m. UTC | #2
On 3/6/19 11:32 AM, Pedro Alves wrote:
> On 02/27/2019 08:18 PM, Tom Tromey wrote:
>> This removes the last cleanups from solib-svr4.c, replacing them with
>> uses of make_scope_exit.
>>
>> gdb/ChangeLog
>> 2019-02-27  Tom Tromey  <tom@tromey.com>
>>
>> 	* solib-svr4.c (svr4_parse_libraries, svr4_current_sos_direct):
>> 	Use make_scope_exit.
>> ---
>>  gdb/ChangeLog    |  5 +++++
>>  gdb/solib-svr4.c | 17 ++++++++++-------
>>  2 files changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
>> index 2b370ef96d0..2301cf94c2f 100644
>> --- a/gdb/solib-svr4.c
>> +++ b/gdb/solib-svr4.c
>> @@ -1198,8 +1198,10 @@ static const struct gdb_xml_element svr4_library_list_elements[] =
>>  static int
>>  svr4_parse_libraries (const char *document, struct svr4_library_list *list)
>>  {
>> -  struct cleanup *back_to = make_cleanup (svr4_free_library_list,
>> -					  &list->head);
>> +  auto cleanup = make_scope_exit ([&] ()
>> +				  {
>> +				    svr4_free_library_list (&list->head);
>> +				  });
>>  
> 
> When passing lambdas as last argument to a function, I think indenting
> like this reads clearer:
> 
>   auto cleanup = make_scope_exit ([&] ()
>     {
>       svr4_free_library_list (&list->head);
>     });
> 
> Makes it read more like an if/for scope block, and of course,
> avoids aligning things too much to the right side.  Examples of
> this can be found throughout gdbserver, for example:
> 
> 	  for_each_thread ([] (thread_info *thread)
> 	    {
> 	      thread->status_pending_p = 0;
> 	    });
> 
> and, also, that's how you'd indent if you used SCOPE_EXIT instead:
> 
>   SCOPE_EXIT
>     {
>       svr4_free_library_list (&list->head);
>     }
> 
> LLVM has a written rule for this:
> 
>  https://llvm.org/docs/CodingStandards.html#format-lambdas-like-blocks-of-code
> 
> If others agree, I'd vote for having a similar rule of our own.

I'd +1 that rule FWIW.
Tom Tromey March 6, 2019, 9:17 p.m. UTC | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> When passing lambdas as last argument to a function, I think indenting
Pedro> like this reads clearer:

Pedro>   auto cleanup = make_scope_exit ([&] ()
Pedro>     {
Pedro>       svr4_free_library_list (&list->head);
Pedro>     });

I agree.  I made this change here & in another patch in the series.

Pedro> If others agree, I'd vote for having a similar rule of our own.

Let's do it.

Tom
diff mbox

Patch

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 2b370ef96d0..2301cf94c2f 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1198,8 +1198,10 @@  static const struct gdb_xml_element svr4_library_list_elements[] =
 static int
 svr4_parse_libraries (const char *document, struct svr4_library_list *list)
 {
-  struct cleanup *back_to = make_cleanup (svr4_free_library_list,
-					  &list->head);
+  auto cleanup = make_scope_exit ([&] ()
+				  {
+				    svr4_free_library_list (&list->head);
+				  });
 
   memset (list, 0, sizeof (*list));
   list->tailp = &list->head;
@@ -1207,11 +1209,10 @@  svr4_parse_libraries (const char *document, struct svr4_library_list *list)
 			   svr4_library_list_elements, document, list) == 0)
     {
       /* Parsed successfully, keep the result.  */
-      discard_cleanups (back_to);
+      cleanup.release ();
       return 1;
     }
 
-  do_cleanups (back_to);
   return 0;
 }
 
@@ -1374,7 +1375,6 @@  svr4_current_sos_direct (struct svr4_info *info)
   CORE_ADDR lm;
   struct so_list *head = NULL;
   struct so_list **link_ptr = &head;
-  struct cleanup *back_to;
   int ignore_first;
   struct svr4_library_list library_list;
 
@@ -1412,7 +1412,10 @@  svr4_current_sos_direct (struct svr4_info *info)
   else
     ignore_first = 1;
 
-  back_to = make_cleanup (svr4_free_library_list, &head);
+  auto cleanup = make_scope_exit ([&] ()
+				  {
+				    svr4_free_library_list (&head);
+				  });
 
   /* Walk the inferior's link map list, and build our list of
      `struct so_list' nodes.  */
@@ -1428,7 +1431,7 @@  svr4_current_sos_direct (struct svr4_info *info)
   if (lm)
     svr4_read_so_list (lm, 0, &link_ptr, 0);
 
-  discard_cleanups (back_to);
+  cleanup.release ();
 
   if (head == NULL)
     return svr4_default_sos ();