[PATCH/doc] Remove fixme of packet "k"

Message ID 5322AE41.4050008@mentor.com
State Superseded
Headers

Commit Message

Hui Zhu March 14, 2014, 7:22 a.m. UTC
  Current introduction of 'k' is:
‘k’
Kill request.
FIXME: There is no description of how to operate when a specific thread context has been selected (i.e. does 'k' kill only that thread?).

I checked the code about this part:
In GDB side:
static void
remote_kill (struct target_ops *ops)
{
   volatile struct gdb_exception ex;

   /* Catch errors so the user can quit from gdb even when we
      aren't on speaking terms with the remote system.  */
   TRY_CATCH (ex, RETURN_MASK_ERROR)
     {
       putpkt ("k");
     }
   if (ex.reason < 0)
     {
       if (ex.error == TARGET_CLOSE_ERROR)
	{
	  /* If we got an (EOF) error that caused the target
	     to go away, then we're done, that's what we wanted.
	     "k" is susceptible to cause a premature EOF, given
	     that the remote server isn't actually required to
	     reply to "k", and it can happen that it doesn't
	     even get to reply ACK to the "k".  */
	  return;
	}

	/* Otherwise, something went wrong.  We didn't actually kill
	   the target.  Just propagate the exception, and let the
	   user o:r higher layers decide what to do.  */
	throw_exception (ex);
     }

   /* We've killed the remote end, we get to mourn it.  Since this is
      target remote, single-process, mourning the inferior also
      unpushes remote_ops.  */
   target_mourn_inferior ();
}

static void
extended_remote_kill (struct target_ops *ops)
{
   int res;
   int pid = ptid_get_pid (inferior_ptid);
   struct remote_state *rs = get_remote_state ();

   res = remote_vkill (pid, rs);
   if (res == -1 && !(rs->extended && remote_multi_process_p (rs)))
     {
       /* Don't try 'k' on a multi-process aware stub -- it has no way
	 to specify the pid.  */

       putpkt ("k");
#if 0
       getpkt (&rs->buf, &rs->buf_size, 0);
       if (rs->buf[0] != 'O' || rs->buf[0] != 'K')
	res = 1;
#else
       /* Don't wait for it to die.  I'm not really sure it matters whether
	 we do or not.  For the existing stubs, kill is a noop.  */
       res = 0;
#endif
     }

   if (res != 0)
     error (_("Can't kill process"));

   target_mourn_inferior ();
}

In gdbserver side:
      fprintf (stderr, "Killing all inferiors\n");
       for_each_inferior (&all_processes, kill_inferior_callback);

       /* When using the extended protocol, we wait with no program
	 running.  The traditional protocol will exit instead.  */
       if (extended_protocol)
	{
	  last_status.kind = TARGET_WAITKIND_EXITED;
	  last_status.value.sig = GDB_SIGNAL_KILL;
	  return 0;
	}
       else
	exit (0);

So make a patch update doc of 'k' to:
‘k’
Kill all processes.

The ‘k’ packet has no reply.

Thanks,
Hui

2014-03-14  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (Packets): Update introduction of 'k'.
  

Comments

Eli Zaretskii March 14, 2014, 8:14 a.m. UTC | #1
> Date: Fri, 14 Mar 2014 15:22:41 +0800
> From: Hui Zhu <hui_zhu@mentor.com>
> CC: Eli Zaretskii <eliz@gnu.org>
> 
> So make a patch update doc of 'k' to:
> ‘k’
> Kill all processes.
> 
> The ‘k’ packet has no reply.

Fine with me, assuming that someone who knows that code will confirm
your understanding.
  
Stan Shebs March 18, 2014, 8:52 p.m. UTC | #2
On 3/14/14 12:22 AM, Hui Zhu wrote:

[...]
> So make a patch update doc of 'k' to:
> ‘k’
> Kill all processes.
> 
> The ‘k’ packet has no reply.

For old packets in the protocol, we need to be careful not to tweak the
description so as to change the meaning.  In this case, the effect of
'k' is not precisely specified; while the current version of GDBserver
has taken it to mean "all inferiors", that may not be true of the stub
I shipped to a customer four years ago.

As a replacement for the FIXME, I suggest something like

"Kill the target process or processes."

"The exact effect of this packet is not specified.  For a single-process
target, it will kill that process if possible.  A multiple-process
target may choose to kill just one process, or all that that
are under GDB's control.  For more precise control, use the vKill packet."

"The ‘k’ packet has no reply."

It seems like it should say something about inferiors as well, but
I couldn't think of how to express it clearly.

Stan
stan@codesourcery.com
  

Patch

--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33952,11 +33952,9 @@  step packet}.
  
  @item k
  @cindex @samp{k} packet
-Kill request.
+Kill all processes.
  
-FIXME: @emph{There is no description of how to operate when a specific
-thread context has been selected (i.e.@: does 'k' kill only that
-thread?)}.
+The @samp{k} packet has no reply.
  
  @item m @var{addr},@var{length}
  @cindex @samp{m} packet