[v2,07/22] Remove last cleanups from solib-svr4.c
Commit Message
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
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
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.
>>>>> "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
@@ -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 ();