[PATCH] gdb/linux-fork: simplify one_fork_p

[PATCH] gdb/linux-fork: simplify one_fork_p

[Simon Marchi]

Unless I'm missing something, this function is a complicated way of
saying "fork_list.size () == 1".

gdb/ChangeLog:

	* linux-fork.c (one_fork_p): Simplify.
---
 gdb/linux-fork.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 284f1985d0dc..357188685d07 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -110,8 +110,7 @@ find_last_fork (void)
 static bool
 one_fork_p ()
 {
-  return (!fork_list.empty ()
-	  && &fork_list.front () == &fork_list.back ());
+  return fork_list.size () == 1;
 }
 
 /* Add a new fork to the internal fork list.  */
-- 
2.24.1

Re: [PATCH] gdb/linux-fork: simplify one_fork_p

[Christian Biesinger via gdb-patches]

On Sun, Jan 19, 2020 at 11:11 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> Unless I'm missing something, this function is a complicated way of
> saying "fork_list.size () == 1".

Before C++11, size() wasn't guaranteed to run in constant time, so I
assume the code was written to handle that. But GDB uses C++11, so
this change seems fine.
https://en.cppreference.com/w/cpp/container/list/size

>
> gdb/ChangeLog:
>
>         * linux-fork.c (one_fork_p): Simplify.
> ---
>  gdb/linux-fork.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
> index 284f1985d0dc..357188685d07 100644
> --- a/gdb/linux-fork.c
> +++ b/gdb/linux-fork.c
> @@ -110,8 +110,7 @@ find_last_fork (void)
>  static bool
>  one_fork_p ()
>  {
> -  return (!fork_list.empty ()
> -         && &fork_list.front () == &fork_list.back ());
> +  return fork_list.size () == 1;
>  }
>
>  /* Add a new fork to the internal fork list.  */
> --
> 2.24.1
>

Re: [PATCH] gdb/linux-fork: simplify one_fork_p

[Simon Marchi]

On 2020-01-19 11:41 a.m., Christian Biesinger wrote:
> On Sun, Jan 19, 2020 at 11:11 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>
>> Unless I'm missing something, this function is a complicated way of
>> saying "fork_list.size () == 1".
> 
> Before C++11, size() wasn't guaranteed to run in constant time, so I
> assume the code was written to handle that. But GDB uses C++11, so
> this change seems fine.
> https://en.cppreference.com/w/cpp/container/list/size

Ahh, good point.  Although by the time that change was made, we were already
using C++11.  I don't remember if we had a C++ < 11 phase, but if we did it
was very short.

Thanks for looking at it, I'll push it now.

Simon

Re: [PATCH] gdb/linux-fork: simplify one_fork_p

[Christian Biesinger via gdb-patches]

On Sun, Jan 19, 2020 at 11:53 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2020-01-19 11:41 a.m., Christian Biesinger wrote:
> > On Sun, Jan 19, 2020 at 11:11 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> >>
> >> Unless I'm missing something, this function is a complicated way of
> >> saying "fork_list.size () == 1".
> >
> > Before C++11, size() wasn't guaranteed to run in constant time, so I
> > assume the code was written to handle that. But GDB uses C++11, so
> > this change seems fine.
> > https://en.cppreference.com/w/cpp/container/list/size
>
> Ahh, good point.  Although by the time that change was made, we were already
> using C++11.  I don't remember if we had a C++ < 11 phase, but if we did it
> was very short.
>
> Thanks for looking at it, I'll push it now.

Ah. it's also possible that whoever wrote the code just assumed that
size() would run in linear time, of course.

Christian

Re: [PATCH] gdb/linux-fork: simplify one_fork_p

[Pedro Alves]

On 1/19/20 4:56 PM, Christian Biesinger via gdb-patches wrote:
> On Sun, Jan 19, 2020 at 11:53 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>
>> On 2020-01-19 11:41 a.m., Christian Biesinger wrote:
>>> On Sun, Jan 19, 2020 at 11:11 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>>>
>>>> Unless I'm missing something, this function is a complicated way of
>>>> saying "fork_list.size () == 1".
>>>
>>> Before C++11, size() wasn't guaranteed to run in constant time, so I
>>> assume the code was written to handle that. But GDB uses C++11, so
>>> this change seems fine.
>>> https://en.cppreference.com/w/cpp/container/list/size
>>
>> Ahh, good point.  Although by the time that change was made, we were already
>> using C++11.  I don't remember if we had a C++ < 11 phase, but if we did it
>> was very short.

Yes, we had one.  It was short.  Everyone hated my unique_ptr emulation
so much that we moved quickly to C++11. :-D

>>
>> Thanks for looking at it, I'll push it now.
> 
> Ah. it's also possible that whoever wrote the code just assumed that
> size() would run in linear time, of course.

Note, I believe that size() isn't linear when compiled with gcc 4.8,
since the new C++11 ABI was only introduced in GCC 5:

 https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/

But I think it's OK to ignore that.  Especially for non-hot code like
here.  I think it's reasonable to say that if you care about performance,
you'll want to compile with a newer compiler.

I was the one who wrote it (06974e6c05556e), but I don't remember why
I did it that way.  Might have been the non-O(1) issue, or it could have been
about blindly C++-fying code without realizing the potential simplification.  
I agree that size () == 1 works just as well, assuming C++11 std::list.

Thanks,
Pedro Alves