Re: [PATCH] Enable multi process debugging for AIX

[Ulrich Weigand <Ulrich.Weigand@de.ibm.com>]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>I have made all the changes you mentioned for optimisation. Please find attached the patch.

Unfortunately it looks like this introduced a bug to has_my_aix_child_reported,
see below.  Otherwise, there's only a few coding style issues left, then this
is ready to merge.


>+static pid_t
>+find_my_aix_parent (pid_t child_pid)
>+{
>+  struct procsinfo ProcessBuffer1;
>+  pid_t IndexPointer1 = child_pid;
>+  int Count = 1;
>+  
>+  if (getprocs (&ProcessBuffer1, sizeof (ProcessBuffer1),
>+      NULL, 0, &IndexPointer1, Count) != 1)

You can just use  ... &child_pid, 1 ... here, then you don't
need the separate variables.

>+static pid_t
>+has_my_aix_child_reported (pid_t parent_pid)
>+{
>+  pid_t child = 0;
>+  auto it = std::find_if (aix_pending_children.begin (),
>+                          aix_pending_children.end (),
>+                          find_my_aix_parent);

This is now wrong, unfortunately.  This only checks that
"find_my_aix_parent" returns any non-null value, but it
actually needs to check that it returns "parent_pid".

This should be something along the lines of:
  auto it = std::find_if (aix_pending_children.begin (),
                          aix_pending_children.end (),
                          [=] (pid_t child_pid)
                          {
                            return find_my_aix_parent (child_pid) == parent_pid;
                          });

>+has_my_aix_parent_reported (pid_t child_pid)
>+{
>+  pid_t my_parent = find_my_aix_parent (child_pid);
>+  auto it = std::find (aix_pending_parent.begin (),
>+                       aix_pending_parent.end (),
>+                       my_parent);
>+  if (it != aix_pending_parent.end ())
>+  {
>+    my_parent = *it;

This line shouldn't be necessary.

>       /* Ignore terminated detached child processes.  */
>       if (!WIFSTOPPED (status) && find_inferior_pid (this, pid) == nullptr)
>-	pid = -1;
>+        continue;	

Here (and elsewhere) make sure to use a tab instead of 8 spaces.
Also, there should be no whitespace at the end of the line.

>+      if (pid != -1)
>+      {

This if is now no longer necessary.

>+      else
>+        break;
>     }
>-  while (pid == -1);
>-
>+  }

Whitespace (indentation) looks wrong.

>+  /* 0x7f is signal 0.  */
>+  /* 0x17f and 0x137f in hexadecimal are status returned if
>+     if we follow parent,
>+     a switch is made to a child post parent execution
>+     and child continues its execution [user switches to child and
>+     presses continue].  */

Use a single comment block and try to make the lines of about
equal length if possible.

Bye,
Ulrich