[PATCH 0/4] Unify getpkt methods in remote.c

[PATCH 0/4] Unify getpkt methods in remote.c

[Tom Tromey]

This series unifies the various getpkt methods in remote.c, and
applies a little bit of bool-ification as well.

---
Tom Tromey (4):
      Remove getpkt_sane
      Remove expecting_notif parameter from getpkt_or_notif_sane_1
      Use bool in getpkt
      Unify getpkt and getpkt_or_notif_sane

 gdb/remote.c | 242 +++++++++++++++++++++++++----------------------------------
 1 file changed, 103 insertions(+), 139 deletions(-)
---
base-commit: b8a175b415454df6a039ba0b5d2ff13c3c180275
change-id: 20230828-getpkt-cleanup-126c625821bf

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/4] Remove getpkt_sane

[Tom Tromey]

I noticed that getpkt is just a wrapper around getpk_sane, so this
patch unifies the two of them.
---
 gdb/remote.c | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 5af40bd704c..29aff714ca5 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1213,10 +1213,9 @@ class remote_target : public process_stratum_target
 
   void skip_frame ();
   long read_frame (gdb::char_vector *buf_p);
-  void getpkt (gdb::char_vector *buf, int forever);
   int getpkt_or_notif_sane_1 (gdb::char_vector *buf, int forever,
 			      int expecting_notif, int *is_notif);
-  int getpkt_sane (gdb::char_vector *buf, int forever);
+  int getpkt (gdb::char_vector *buf, int forever);
   int getpkt_or_notif_sane (gdb::char_vector *buf, int forever,
 			    int *is_notif);
   int remote_vkill (int pid);
@@ -10044,23 +10043,6 @@ show_watchdog (struct ui_file *file, int from_tty,
   gdb_printf (file, _("Watchdog timer is %s.\n"), value);
 }
 
-/* Read a packet from the remote machine, with error checking, and
-   store it in *BUF.  Resize *BUF if necessary to hold the result.  If
-   FOREVER, wait forever rather than timing out; this is used (in
-   synchronous mode) to wait for a target that is is executing user
-   code to stop.  */
-/* FIXME: ezannoni 2000-02-01 this wrapper is necessary so that we
-   don't have to change all the calls to getpkt to deal with the
-   return value, because at the moment I don't know what the right
-   thing to do it for those.  */
-
-void
-remote_target::getpkt (gdb::char_vector *buf, int forever)
-{
-  getpkt_sane (buf, forever);
-}
-
-
 /* Read a packet from the remote machine, with error checking, and
    store it in *BUF.  Resize *BUF if necessary to hold the result.  If
    FOREVER, wait forever rather than timing out; this is used (in
@@ -10212,8 +10194,14 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
     }
 }
 
+/* Read a packet from the remote machine, with error checking, and
+   store it in *BUF.  Resize *BUF if necessary to hold the result.  If
+   FOREVER, wait forever rather than timing out; this is used (in
+   synchronous mode) to wait for a target that is is executing user
+   code to stop.  */
+
 int
-remote_target::getpkt_sane (gdb::char_vector *buf, int forever)
+remote_target::getpkt (gdb::char_vector *buf, int forever)
 {
   return getpkt_or_notif_sane_1 (buf, forever, 0, NULL);
 }
@@ -11289,7 +11277,7 @@ remote_target::remote_write_qxfer (const char *object_name,
     (writebuf, len, 1, (gdb_byte *) rs->buf.data () + i, &max_size, max_size);
 
   if (putpkt_binary (rs->buf.data (), i + buf_len) < 0
-      || getpkt_sane (&rs->buf, 0) < 0
+      || getpkt (&rs->buf, 0) < 0
       || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
     return TARGET_XFER_E_IO;
 
@@ -11353,7 +11341,7 @@ remote_target::remote_read_qxfer (const char *object_name,
     return TARGET_XFER_E_IO;
 
   rs->buf[0] = '\0';
-  packet_len = getpkt_sane (&rs->buf, 0);
+  packet_len = getpkt (&rs->buf, 0);
   if (packet_len < 0
       || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
     return TARGET_XFER_E_IO;
@@ -11658,7 +11646,7 @@ remote_target::search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
     error (_("Pattern is too large to transmit to remote target."));
 
   if (putpkt_binary (rs->buf.data (), i + escaped_pattern_len) < 0
-      || getpkt_sane (&rs->buf, 0) < 0
+      || getpkt (&rs->buf, 0) < 0
       || m_features.packet_ok (rs->buf, PACKET_qSearch_memory) != PACKET_OK)
     {
       /* The request may not have worked because the command is not
@@ -11722,7 +11710,7 @@ remote_target::rcmd (const char *command, struct ui_file *outbuf)
       /* XXX - see also remote_get_noisy_reply().  */
       QUIT;			/* Allow user to bail out with ^C.  */
       rs->buf[0] = '\0';
-      if (getpkt_sane (&rs->buf, 0) == -1)
+      if (getpkt (&rs->buf, 0) == -1)
 	{ 
 	  /* Timeout.  Continue to (try to) read responses.
 	     This is better than stopping with an error, assuming the stub
@@ -11833,7 +11821,7 @@ send_remote_packet (gdb::array_view<const char> &buf,
 
   remote->putpkt_binary (buf.data (), buf.size ());
   remote_state *rs = remote->get_remote_state ();
-  int bytes = remote->getpkt_sane (&rs->buf, 0);
+  int bytes = remote->getpkt (&rs->buf, 0);
 
   if (bytes < 0)
     error (_("error while fetching packet from remote target"));
@@ -12366,7 +12354,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
     }
 
   putpkt_binary (rs->buf.data (), command_bytes);
-  bytes_read = getpkt_sane (&rs->buf, 0);
+  bytes_read = getpkt (&rs->buf, 0);
 
   /* If it timed out, something is wrong.  Don't try to parse the
      buffer.  */

-- 
2.40.1

[PATCH 2/4] Remove expecting_notif parameter from getpkt_or_notif_sane_1

[Tom Tromey]

For getpkt_or_notif_sane_1, expecting_notif is redundant, because it
always reflects whether the is_notif parameter is non-NULL.  This
patch removes the redundant parameter.
---
 gdb/remote.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 29aff714ca5..5f40586b852 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1214,7 +1214,7 @@ class remote_target : public process_stratum_target
   void skip_frame ();
   long read_frame (gdb::char_vector *buf_p);
   int getpkt_or_notif_sane_1 (gdb::char_vector *buf, int forever,
-			      int expecting_notif, int *is_notif);
+			      int *is_notif);
   int getpkt (gdb::char_vector *buf, int forever);
   int getpkt_or_notif_sane (gdb::char_vector *buf, int forever,
 			    int *is_notif);
@@ -10049,14 +10049,15 @@ show_watchdog (struct ui_file *file, int from_tty,
    synchronous mode) to wait for a target that is is executing user
    code to stop.  If FOREVER == 0, this function is allowed to time
    out gracefully and return an indication of this to the caller.
-   Otherwise return the number of bytes read.  If EXPECTING_NOTIF,
-   consider receiving a notification enough reason to return to the
-   caller.  *IS_NOTIF is an output boolean that indicates whether *BUF
-   holds a notification or not (a regular packet).  */
+   Otherwise return the number of bytes read.  If IS_NOTIF is not
+   NULL, then consider receiving a notification enough reason to
+   return to the caller.  In this case, *IS_NOTIF is an output boolean
+   that indicates whether *BUF holds a notification or not (a regular
+   packet).  */
 
 int
 remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
-				       int forever, int expecting_notif,
+				       int forever,
 				       int *is_notif)
 {
   struct remote_state *rs = get_remote_state ();
@@ -10069,7 +10070,7 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 
   if (forever)
     timeout = watchdog > 0 ? watchdog : -1;
-  else if (expecting_notif)
+  else if (is_notif != nullptr)
     timeout = 0; /* There should already be a char in the buffer.  If
 		    not, bail out.  */
   else
@@ -10100,7 +10101,7 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 
 	  if (c == SERIAL_TIMEOUT)
 	    {
-	      if (expecting_notif)
+	      if (is_notif != nullptr)
 		return -1; /* Don't complain, it's normal to not get
 			      anything in this case.  */
 
@@ -10188,7 +10189,7 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 
 	  /* Notifications require no acknowledgement.  */
 
-	  if (expecting_notif)
+	  if (is_notif != nullptr)
 	    return val;
 	}
     }
@@ -10203,14 +10204,14 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 int
 remote_target::getpkt (gdb::char_vector *buf, int forever)
 {
-  return getpkt_or_notif_sane_1 (buf, forever, 0, NULL);
+  return getpkt_or_notif_sane_1 (buf, forever, NULL);
 }
 
 int
 remote_target::getpkt_or_notif_sane (gdb::char_vector *buf, int forever,
 				     int *is_notif)
 {
-  return getpkt_or_notif_sane_1 (buf, forever, 1, is_notif);
+  return getpkt_or_notif_sane_1 (buf, forever, is_notif);
 }
 
 /* Kill any new fork children of inferior INF that haven't been

-- 
2.40.1

[PATCH 3/4] Use bool in getpkt

[Tom Tromey]

This changes getpkt and related functions to use bool rather than int.
---
 gdb/remote.c | 204 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 102 insertions(+), 102 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 5f40586b852..88c422b0c63 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -551,7 +551,7 @@ class remote_state
      modified to return a timeout indication and, in turn
      remote_wait()/wait_for_inferior() have gained a timeout parameter
      this can go away.  */
-  int wait_forever_enabled_p = 1;
+  bool wait_forever_enabled_p = true;
 
 private:
   /* Mapping of remote protocol data for each gdbarch.  Usually there
@@ -1213,11 +1213,11 @@ class remote_target : public process_stratum_target
 
   void skip_frame ();
   long read_frame (gdb::char_vector *buf_p);
-  int getpkt_or_notif_sane_1 (gdb::char_vector *buf, int forever,
-			      int *is_notif);
-  int getpkt (gdb::char_vector *buf, int forever);
-  int getpkt_or_notif_sane (gdb::char_vector *buf, int forever,
-			    int *is_notif);
+  int getpkt_or_notif_sane_1 (gdb::char_vector *buf, bool forever,
+			      bool *is_notif);
+  int getpkt (gdb::char_vector *buf, bool forever);
+  int getpkt_or_notif_sane (gdb::char_vector *buf, bool forever,
+			    bool *is_notif);
   int remote_vkill (int pid);
   void remote_kill_k ();
 
@@ -1561,7 +1561,7 @@ remote_target::remote_get_noisy_reply ()
       char *buf;
 
       QUIT;			/* Allow user to bail out with ^C.  */
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       buf = rs->buf.data ();
       if (buf[0] == 'E')
 	trace_error (buf);
@@ -2627,7 +2627,7 @@ remote_target::remote_query_attached (int pid)
     xsnprintf (rs->buf.data (), size, "qAttached");
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_qAttached))
     {
@@ -2925,7 +2925,7 @@ remote_target::pass_signals (gdb::array_view<const unsigned char> pass_signals)
       if (!rs->last_pass_packet || strcmp (rs->last_pass_packet, pass_packet))
 	{
 	  putpkt (pass_packet);
-	  getpkt (&rs->buf, 0);
+	  getpkt (&rs->buf, false);
 	  m_features.packet_ok (rs->buf, PACKET_QPassSignals);
 	  xfree (rs->last_pass_packet);
 	  rs->last_pass_packet = pass_packet;
@@ -2998,7 +2998,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
   struct remote_state *rs = get_remote_state ();
 
   putpkt (catch_packet);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   result = m_features.packet_ok (rs->buf, PACKET_QCatchSyscalls);
   if (result == PACKET_OK)
     return 0;
@@ -3046,7 +3046,7 @@ remote_target::program_signals (gdb::array_view<const unsigned char> signals)
 	  || strcmp (rs->last_program_signals_packet, packet) != 0)
 	{
 	  putpkt (packet);
-	  getpkt (&rs->buf, 0);
+	  getpkt (&rs->buf, false);
 	  m_features.packet_ok (rs->buf, PACKET_QProgramSignals);
 	  xfree (rs->last_program_signals_packet);
 	  rs->last_program_signals_packet = packet;
@@ -3082,7 +3082,7 @@ remote_target::set_thread (ptid_t ptid, int gen)
   else
     write_ptid (buf, endbuf, ptid);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (gen)
     rs->general_thread = ptid;
   else
@@ -3166,7 +3166,7 @@ remote_target::thread_alive (ptid_t ptid)
   write_ptid (p, endp, ptid);
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   return (rs->buf[0] == 'O' && rs->buf[1] == 'K');
 }
 
@@ -3666,7 +3666,7 @@ remote_target::remote_get_threadinfo (threadref *threadid,
 
   pack_threadinfo_request (rs->buf.data (), fieldset, threadid);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   if (rs->buf[0] == '\0')
     return 0;
@@ -3740,7 +3740,7 @@ remote_target::remote_get_threadlist (int startflag, threadref *nextthread,
   pack_threadlist_request (rs->buf.data (), startflag, result_limit,
 			   nextthread);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (rs->buf[0] == '\0')
     {
       /* Packet not supported.  */
@@ -3927,7 +3927,7 @@ remote_target::remote_current_thread (ptid_t oldpid)
   struct remote_state *rs = get_remote_state ();
 
   putpkt ("qC");
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (rs->buf[0] == 'Q' && rs->buf[1] == 'C')
     {
       const char *obuf;
@@ -4061,7 +4061,7 @@ remote_target::remote_get_threads_with_qthreadinfo (threads_listing_context *con
       const char *bufp;
 
       putpkt ("qfThreadInfo");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       bufp = rs->buf.data ();
       if (bufp[0] != '\0')		/* q packet recognized */
 	{
@@ -4074,7 +4074,7 @@ remote_target::remote_get_threads_with_qthreadinfo (threads_listing_context *con
 		}
 	      while (*bufp++ == ',');	/* comma-separated list */
 	      putpkt ("qsThreadInfo");
-	      getpkt (&rs->buf, 0);
+	      getpkt (&rs->buf, false);
 	      bufp = rs->buf.data ();
 	    }
 	  return 1;
@@ -4241,7 +4241,7 @@ remote_target::extra_thread_info (thread_info *tp)
       write_ptid (b, endb, tp->ptid);
 
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       if (rs->buf[0] != 0)
 	{
 	  extra.resize (strlen (rs->buf.data ()) / 2);
@@ -4289,7 +4289,7 @@ remote_target::static_tracepoint_marker_at (CORE_ADDR addr,
   p += strlen (p);
   p += hexnumstr (p, addr);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   p = rs->buf.data ();
 
   if (*p == 'E')
@@ -4315,7 +4315,7 @@ remote_target::static_tracepoint_markers_by_strid (const char *strid)
   /* Ask for a first packet of static tracepoint marker
      definition.  */
   putpkt ("qTfSTM");
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   p = rs->buf.data ();
   if (*p == 'E')
     error (_("Remote failure reply: %s"), p);
@@ -4332,7 +4332,7 @@ remote_target::static_tracepoint_markers_by_strid (const char *strid)
       while (*p++ == ',');	/* comma-separated list */
       /* Ask for another packet of static tracepoint definition.  */
       putpkt ("qTsSTM");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       p = rs->buf.data ();
     }
 
@@ -4413,7 +4413,7 @@ remote_target::get_offsets ()
     return;
 
   putpkt ("qOffsets");
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   buf = rs->buf.data ();
 
   if (buf[0] == '\000')
@@ -4962,7 +4962,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
     const char v_mustreplyempty[] = "vMustReplyEmpty";
 
     putpkt (v_mustreplyempty);
-    getpkt (&rs->buf, 0);
+    getpkt (&rs->buf, false);
     if (strcmp (rs->buf.data (), "OK") == 0)
       {
 	m_features.m_protocol_packets[PACKET_vFile_setfs].support
@@ -4989,7 +4989,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
   if (m_features.packet_support (PACKET_QStartNoAckMode) != PACKET_DISABLE)
     {
       putpkt ("QStartNoAckMode");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       if (m_features.packet_ok (rs->buf, PACKET_QStartNoAckMode) == PACKET_OK)
 	rs->noack_mode = 1;
     }
@@ -4998,7 +4998,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
     {
       /* Tell the remote that we are using the extended protocol.  */
       putpkt ("!");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
     }
 
   /* Let the target know which signals it is allowed to pass down to
@@ -5025,7 +5025,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
 		 "does not support non-stop"));
 
       putpkt ("QNonStop:1");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       if (strcmp (rs->buf.data (), "OK") != 0)
 	error (_("Remote refused setting non-stop mode with: %s"),
@@ -5042,7 +5042,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
       /* Don't assume that the stub can operate in all-stop mode.
 	 Request it explicitly.  */
       putpkt ("QNonStop:0");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       if (strcmp (rs->buf.data (), "OK") != 0)
 	error (_("Remote refused setting all-stop mode with: %s"),
@@ -5062,7 +5062,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
 
   /* Check whether the target is running now.  */
   putpkt ("?");
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   if (!target_is_non_stop_p ())
     {
@@ -5329,7 +5329,7 @@ remote_target::remote_check_symbols ()
   /* Invite target to request symbol lookups.  */
 
   putpkt ("qSymbol::");
-  getpkt (&reply, 0);
+  getpkt (&reply, false);
   m_features.packet_ok (reply, PACKET_qSymbol);
 
   while (startswith (reply.data (), "qSymbol:"))
@@ -5359,7 +5359,7 @@ remote_target::remote_check_symbols ()
 	}
 
       putpkt (msg.data ());
-      getpkt (&reply, 0);
+      getpkt (&reply, false);
     }
 }
 
@@ -5405,7 +5405,7 @@ remote_target::set_permissions ()
 	     may_insert_breakpoints, may_insert_tracepoints,
 	     may_insert_fast_tracepoints, may_stop);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   /* If the target didn't like the packet, warn the user.  Do not try
      to undo the user's settings, that would just be maddening.  */
@@ -5718,7 +5718,7 @@ remote_target::remote_query_supported ()
       q = "qSupported:" + q;
       putpkt (q.c_str ());
 
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       /* If an error occurred, warn, but do not return - just reset the
 	 buffer to empty and go on to disable features.  */
@@ -5954,7 +5954,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
 
   /* See FIXME above.  */
   if (!target_async_permitted)
-    rs->wait_forever_enabled_p = 1;
+    rs->wait_forever_enabled_p = true;
 
   rs->remote_desc = remote_serial_open (name);
   if (!rs->remote_desc)
@@ -6028,7 +6028,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
 	 around this.  Eventually a mechanism that allows
 	 wait_for_inferior() to expect/get timeouts will be
 	 implemented.  */
-      rs->wait_forever_enabled_p = 0;
+      rs->wait_forever_enabled_p = false;
     }
 
   /* First delete any symbols previously loaded from shared libraries.  */
@@ -6068,7 +6068,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
   remote_btrace_reset (rs);
 
   if (target_async_permitted)
-    rs->wait_forever_enabled_p = 1;
+    rs->wait_forever_enabled_p = true;
 }
 
 /* Determine if WS represents a fork status.  */
@@ -6118,7 +6118,7 @@ remote_target::remote_detach_pid (int pid)
     strcpy (rs->buf.data (), "D");
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   if (rs->buf[0] == 'O' && rs->buf[1] == 'K')
     ;
@@ -6319,7 +6319,7 @@ extended_remote_target::attach (const char *args, int from_tty)
 
   xsnprintf (rs->buf.data (), get_remote_packet_size (), "vAttach;%x", pid);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_vAttach))
     {
@@ -6422,7 +6422,7 @@ remote_target::remote_vcont_probe ()
 
   strcpy (rs->buf.data (), "vCont?");
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   buf = rs->buf.data ();
 
   /* Make sure that the features we assume are supported.  */
@@ -6701,7 +6701,7 @@ remote_target::remote_resume_with_vcont (ptid_t scope_ptid, int step,
       /* In non-stop, the stub replies to vCont with "OK".  The stop
 	 reply will be reported asynchronously by means of a `%Stop'
 	 notification.  */
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       if (strcmp (rs->buf.data (), "OK") != 0)
 	error (_("Unexpected vCont reply in non-stop mode: %s"),
 	       rs->buf.data ());
@@ -6851,7 +6851,7 @@ vcont_builder::flush ()
 
   rs = m_remote->get_remote_state ();
   m_remote->putpkt (rs->buf);
-  m_remote->getpkt (&rs->buf, 0);
+  m_remote->getpkt (&rs->buf, false);
   if (strcmp (rs->buf.data (), "OK") != 0)
     error (_("Unexpected vCont reply in non-stop mode: %s"), rs->buf.data ());
 }
@@ -7213,7 +7213,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
   /* In non-stop, we get an immediate OK reply.  The stop reply will
      come in asynchronously by notification.  */
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (strcmp (rs->buf.data (), "OK") != 0)
     error (_("Stopping %s failed: %s"), target_pid_to_str (ptid).c_str (),
 	   rs->buf.data ());
@@ -7257,7 +7257,7 @@ remote_target::remote_interrupt_ns ()
   /* In non-stop, we get an immediate OK reply.  The stop reply will
      come in asynchronously by notification.  */
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_vCtrlC))
     {
@@ -8087,7 +8087,7 @@ remote_target::remote_notif_get_pending_events (const notif_client *nc)
 
       while (1)
 	{
-	  getpkt (&rs->buf, 0);
+	  getpkt (&rs->buf, false);
 	  if (strcmp (rs->buf.data (), "OK") == 0)
 	    break;
 	  else
@@ -8292,12 +8292,12 @@ remote_target::wait_ns (ptid_t ptid, struct target_waitstatus *status,
   struct remote_state *rs = get_remote_state ();
   struct stop_reply *stop_reply;
   int ret;
-  int is_notif = 0;
+  bool is_notif = false;
 
   /* If in non-stop mode, get out of getpkt even if a
      notification is received.	*/
 
-  ret = getpkt_or_notif_sane (&rs->buf, 0 /* forever */, &is_notif);
+  ret = getpkt_or_notif_sane (&rs->buf, false /* forever */, &is_notif);
   while (1)
     {
       if (ret != -1 && !is_notif)
@@ -8336,7 +8336,7 @@ remote_target::wait_ns (ptid_t ptid, struct target_waitstatus *status,
 	}
 
       /* Otherwise do a blocking wait.  */
-      ret = getpkt_or_notif_sane (&rs->buf, 1 /* forever */, &is_notif);
+      ret = getpkt_or_notif_sane (&rs->buf, true /* forever */, &is_notif);
     }
 }
 
@@ -8377,8 +8377,8 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
     }
   else
     {
-      int forever = ((options & TARGET_WNOHANG) == 0
-		     && rs->wait_forever_enabled_p);
+      bool forever = ((options & TARGET_WNOHANG) == 0
+		      && rs->wait_forever_enabled_p);
 
       if (!rs->waiting_for_stop_reply)
 	{
@@ -8390,7 +8390,7 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
 	 _never_ wait for ever -> test on target_is_async_p().
 	 However, before we do that we need to ensure that the caller
 	 knows how to take the target into/out of async mode.  */
-      int is_notif;
+      bool is_notif;
       int ret = getpkt_or_notif_sane (&rs->buf, forever, &is_notif);
 
       /* GDB gets a notification.  Return to core as this event is
@@ -8565,7 +8565,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
   p += hexnumstr (p, reg->pnum);
   *p++ = '\0';
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   buf = rs->buf.data ();
 
@@ -8614,7 +8614,7 @@ remote_target::send_g_packet ()
 
   xsnprintf (rs->buf.data (), get_remote_packet_size (), "g");
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (packet_check_result (rs->buf) == PACKET_ERROR)
     error (_("Could not read registers; remote failure reply '%s'"),
 	   rs->buf.data ());
@@ -8628,7 +8628,7 @@ remote_target::send_g_packet ()
 	 && rs->buf[0] != 'x')	/* New: unavailable register value.  */
     {
       remote_debug_printf ("Bad register packet; fetching a new packet");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
     }
 
   buf_len = strlen (rs->buf.data ());
@@ -8873,7 +8873,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
   regcache->raw_collect (reg->regnum, regp);
   bin2hex (regp, p, register_size (gdbarch, reg->regnum));
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_P))
     {
@@ -8922,7 +8922,7 @@ remote_target::store_registers_using_G (const struct regcache *regcache)
   *p++ = 'G';
   bin2hex (regs, p, rsa->sizeof_g_packet);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (packet_check_result (rs->buf) == PACKET_ERROR)
     error (_("Could not write registers; remote failure reply '%s'"), 
 	   rs->buf.data ());
@@ -9077,7 +9077,7 @@ remote_target::check_binary_download (CORE_ADDR addr)
 	*p = '\0';
 
 	putpkt_binary (rs->buf.data (), (int) (p - rs->buf.data ()));
-	getpkt (&rs->buf, 0);
+	getpkt (&rs->buf, false);
 
 	if (rs->buf[0] == '\0')
 	  {
@@ -9281,7 +9281,7 @@ remote_target::remote_write_bytes_aux (const char *header, CORE_ADDR memaddr,
     }
 
   putpkt_binary (rs->buf.data (), (int) (p - rs->buf.data ()));
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   if (rs->buf[0] == 'E')
     return TARGET_XFER_E_IO;
@@ -9373,7 +9373,7 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr,
   p += hexnumstr (p, (ULONGEST) todo_units);
   *p = '\0';
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (rs->buf[0] == 'E'
       && isxdigit (rs->buf[1]) && isxdigit (rs->buf[2])
       && rs->buf[3] == '\0')
@@ -9527,7 +9527,7 @@ remote_target::remote_send_printf (const char *format, ...)
     error (_("Communication problem with target."));
 
   rs->buf[0] = '\0';
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   return packet_check_result (rs->buf);
 }
@@ -10047,7 +10047,7 @@ show_watchdog (struct ui_file *file, int from_tty,
    store it in *BUF.  Resize *BUF if necessary to hold the result.  If
    FOREVER, wait forever rather than timing out; this is used (in
    synchronous mode) to wait for a target that is is executing user
-   code to stop.  If FOREVER == 0, this function is allowed to time
+   code to stop.  If FOREVER == false, this function is allowed to time
    out gracefully and return an indication of this to the caller.
    Otherwise return the number of bytes read.  If IS_NOTIF is not
    NULL, then consider receiving a notification enough reason to
@@ -10057,8 +10057,8 @@ show_watchdog (struct ui_file *file, int from_tty,
 
 int
 remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
-				       int forever,
-				       int *is_notif)
+				       bool forever,
+				       bool *is_notif)
 {
   struct remote_state *rs = get_remote_state ();
   int c;
@@ -10168,7 +10168,7 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 	  if (!rs->noack_mode)
 	    remote_serial_write ("+", 1);
 	  if (is_notif != NULL)
-	    *is_notif = 0;
+	    *is_notif = false;
 	  return val;
 	}
 
@@ -10183,7 +10183,7 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 	     escape_buffer (buf->data (), val).c_str ());
 
 	  if (is_notif != NULL)
-	    *is_notif = 1;
+	    *is_notif = true;
 
 	  handle_notification (rs->notif_state, buf->data ());
 
@@ -10202,14 +10202,14 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
    code to stop.  */
 
 int
-remote_target::getpkt (gdb::char_vector *buf, int forever)
+remote_target::getpkt (gdb::char_vector *buf, bool forever)
 {
   return getpkt_or_notif_sane_1 (buf, forever, NULL);
 }
 
 int
-remote_target::getpkt_or_notif_sane (gdb::char_vector *buf, int forever,
-				     int *is_notif)
+remote_target::getpkt_or_notif_sane (gdb::char_vector *buf, bool forever,
+				     bool *is_notif)
 {
   return getpkt_or_notif_sane_1 (buf, forever, is_notif);
 }
@@ -10318,7 +10318,7 @@ remote_target::remote_vkill (int pid)
   /* Tell the remote target to detach.  */
   xsnprintf (rs->buf.data (), get_remote_packet_size (), "vKill;%x", pid);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_vKill))
     {
@@ -10474,7 +10474,7 @@ remote_target::extended_remote_run (const std::string &args)
   rs->buf[len++] = '\0';
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_vRun))
     {
@@ -10516,7 +10516,7 @@ remote_target::send_environment_packet (const char *action,
 	     "%s:%s", packet, encoded_value.c_str ());
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   if (strcmp (rs->buf.data (), "OK") != 0)
     warning (_("Unable to %s environment variable '%s' on remote."),
 	     action, value);
@@ -10532,7 +10532,7 @@ remote_target::extended_remote_environment_support ()
   if (m_features.packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE)
     {
       putpkt ("QEnvironmentReset");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       if (strcmp (rs->buf.data (), "OK") != 0)
 	warning (_("Unable to reset environment on remote."));
     }
@@ -10582,7 +10582,7 @@ remote_target::extended_remote_set_inferior_cwd ()
 	}
 
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       if (m_features.packet_ok (rs->buf, PACKET_QSetWorkingDir) != PACKET_OK)
 	error (_("\
 Remote replied unexpectedly while setting the inferior's working\n\
@@ -10624,7 +10624,7 @@ extended_remote_target::create_inferior (const char *exec_file,
       xsnprintf (rs->buf.data (), get_remote_packet_size (),
 		 "QStartupWithShell:%d", startup_with_shell ? 1 : 0);
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       if (strcmp (rs->buf.data (), "OK") != 0)
 	error (_("\
 Remote replied unexpectedly while setting startup-with-shell: %s"),
@@ -10754,7 +10754,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
 	remote_add_target_side_commands (gdbarch, bp_tgt, p);
 
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       switch (m_features.packet_ok (rs->buf, PACKET_Z0))
 	{
@@ -10803,7 +10803,7 @@ remote_target::remove_breakpoint (struct gdbarch *gdbarch,
       xsnprintf (p, endbuf - p, ",%d", bp_tgt->kind);
 
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       return (rs->buf[0] == 'E');
     }
@@ -10855,7 +10855,7 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
   xsnprintf (p, endbuf - p, ",%x", len);
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
 					  + to_underlying (packet))))
@@ -10904,7 +10904,7 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
   p += hexnumstr (p, (ULONGEST) addr);
   xsnprintf (p, endbuf - p, ",%x", len);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
 					  + to_underlying (packet))))
@@ -11067,7 +11067,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
     remote_add_target_side_commands (gdbarch, bp_tgt, p);
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_Z1))
     {
@@ -11114,7 +11114,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
   xsnprintf (p, endbuf  - p, ",%x", bp_tgt->kind);
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_Z1))
     {
@@ -11155,7 +11155,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
 	 reply.  */
       host_crc = xcrc32 (data, size, 0xffffffff);
 
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       result = m_features.packet_ok (rs->buf, PACKET_qCRC);
       if (result == PACKET_ERROR)
@@ -11278,7 +11278,7 @@ remote_target::remote_write_qxfer (const char *object_name,
     (writebuf, len, 1, (gdb_byte *) rs->buf.data () + i, &max_size, max_size);
 
   if (putpkt_binary (rs->buf.data (), i + buf_len) < 0
-      || getpkt (&rs->buf, 0) < 0
+      || getpkt (&rs->buf, false) < 0
       || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
     return TARGET_XFER_E_IO;
 
@@ -11342,7 +11342,7 @@ remote_target::remote_read_qxfer (const char *object_name,
     return TARGET_XFER_E_IO;
 
   rs->buf[0] = '\0';
-  packet_len = getpkt (&rs->buf, 0);
+  packet_len = getpkt (&rs->buf, false);
   if (packet_len < 0
       || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
     return TARGET_XFER_E_IO;
@@ -11565,7 +11565,7 @@ remote_target::xfer_partial (enum target_object object,
   if (i < 0)
     return TARGET_XFER_E_IO;
 
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   strcpy ((char *) readbuf, rs->buf.data ());
 
   *xfered_len = strlen ((char *) readbuf);
@@ -11647,7 +11647,7 @@ remote_target::search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
     error (_("Pattern is too large to transmit to remote target."));
 
   if (putpkt_binary (rs->buf.data (), i + escaped_pattern_len) < 0
-      || getpkt (&rs->buf, 0) < 0
+      || getpkt (&rs->buf, false) < 0
       || m_features.packet_ok (rs->buf, PACKET_qSearch_memory) != PACKET_OK)
     {
       /* The request may not have worked because the command is not
@@ -11711,7 +11711,7 @@ remote_target::rcmd (const char *command, struct ui_file *outbuf)
       /* XXX - see also remote_get_noisy_reply().  */
       QUIT;			/* Allow user to bail out with ^C.  */
       rs->buf[0] = '\0';
-      if (getpkt (&rs->buf, 0) == -1)
+      if (getpkt (&rs->buf, false) == -1)
 	{ 
 	  /* Timeout.  Continue to (try to) read responses.
 	     This is better than stopping with an error, assuming the stub
@@ -11822,7 +11822,7 @@ send_remote_packet (gdb::array_view<const char> &buf,
 
   remote->putpkt_binary (buf.data (), buf.size ());
   remote_state *rs = remote->get_remote_state ();
-  int bytes = remote->getpkt (&rs->buf, 0);
+  int bytes = remote->getpkt (&rs->buf, false);
 
   if (bytes < 0)
     error (_("error while fetching packet from remote target"));
@@ -12054,7 +12054,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
       *p++ = '\0';
 
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       result = m_features.packet_ok (rs->buf, PACKET_qGetTLSAddr);
       if (result == PACKET_OK)
 	{
@@ -12096,7 +12096,7 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
       *p++ = '\0';
 
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       result = m_features.packet_ok (rs->buf, PACKET_qGetTIBAddr);
       if (result == PACKET_OK)
 	{
@@ -12355,7 +12355,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
     }
 
   putpkt_binary (rs->buf.data (), command_bytes);
-  bytes_read = getpkt (&rs->buf, 0);
+  bytes_read = getpkt (&rs->buf, false);
 
   /* If it timed out, something is wrong.  Don't try to parse the
      buffer.  */
@@ -13631,7 +13631,7 @@ Too many sections for read-only sections definition packet."));
   if (anysecs)
     {
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
     }
 }
 
@@ -14112,7 +14112,7 @@ remote_target::use_agent (bool use)
       /* If the stub supports QAgent.  */
       xsnprintf (rs->buf.data (), get_remote_packet_size (), "QAgent:%d", use);
       putpkt (rs->buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       if (strcmp (rs->buf.data (), "OK") == 0)
 	{
@@ -14168,7 +14168,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
 			conf->bts.size);
 
       putpkt (buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       if (m_features.packet_ok (buf, PACKET_Qbtrace_conf_bts_size)
 	  == PACKET_ERROR)
@@ -14191,7 +14191,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
 			conf->pt.size);
 
       putpkt (buf);
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
 
       if (m_features.packet_ok (buf, PACKET_Qbtrace_conf_pt_size)
 	  == PACKET_ERROR)
@@ -14315,7 +14315,7 @@ remote_target::enable_btrace (thread_info *tp,
   buf += xsnprintf (buf, endbuf - buf, "%s",
 		    packets_descriptions[which_packet].name);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   if (m_features.packet_ok (rs->buf, which_packet) == PACKET_ERROR)
     {
@@ -14362,7 +14362,7 @@ remote_target::disable_btrace (struct btrace_target_info *tinfo)
   buf += xsnprintf (buf, endbuf - buf, "%s",
 		    packets_descriptions[PACKET_Qbtrace_off].name);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   if (m_features.packet_ok (rs->buf, PACKET_Qbtrace_off) == PACKET_ERROR)
     {
@@ -14638,7 +14638,7 @@ remote_target::thread_events (int enable)
 
   xsnprintf (rs->buf.data (), size, "QThreadEvents:%x", enable ? 1 : 0);
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   switch (m_features.packet_ok (rs->buf, PACKET_QThreadEvents))
     {
@@ -14752,14 +14752,14 @@ remote_target::upload_tracepoints (struct uploaded_tp **utpp)
 
   /* Ask for a first packet of tracepoint definition.  */
   putpkt ("qTfP");
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   p = rs->buf.data ();
   while (*p && *p != 'l')
     {
       parse_tracepoint_definition (p, utpp);
       /* Ask for another packet of tracepoint definition.  */
       putpkt ("qTsP");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       p = rs->buf.data ();
     }
   return 0;
@@ -14773,14 +14773,14 @@ remote_target::upload_trace_state_variables (struct uploaded_tsv **utsvp)
 
   /* Ask for a first packet of variable definition.  */
   putpkt ("qTfV");
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
   p = rs->buf.data ();
   while (*p && *p != 'l')
     {
       parse_tsv_definition (p, utsvp);
       /* Ask for another packet of variable definition.  */
       putpkt ("qTsV");
-      getpkt (&rs->buf, 0);
+      getpkt (&rs->buf, false);
       p = rs->buf.data ();
     }
   return 0;
@@ -14921,7 +14921,7 @@ remote_target::fetch_memtags (CORE_ADDR address, size_t len,
   create_fetch_memtags_request (rs->buf, address, len, type);
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   return parse_fetch_memtags_reply (rs->buf, tags);
 }
@@ -14941,7 +14941,7 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
   create_store_memtags_request (rs->buf, address, len, type, tags);
 
   putpkt (rs->buf);
-  getpkt (&rs->buf, 0);
+  getpkt (&rs->buf, false);
 
   /* Verify if the request was successful.  */
   return packet_check_result (rs->buf.data ()) == PACKET_OK;

-- 
2.40.1

[PATCH 4/4] Unify getpkt and getpkt_or_notif_sane

[Tom Tromey]

getpkt and getpkt_or_notif_sane are just wrappers for
getpkt_or_notif_sane_1.  This patch adds the is_notif parameter to
getpkt, with a suitable default, and removes the wrappers.
---
 gdb/remote.c | 35 +++++------------------------------
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 88c422b0c63..edda9457745 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1213,11 +1213,7 @@ class remote_target : public process_stratum_target
 
   void skip_frame ();
   long read_frame (gdb::char_vector *buf_p);
-  int getpkt_or_notif_sane_1 (gdb::char_vector *buf, bool forever,
-			      bool *is_notif);
-  int getpkt (gdb::char_vector *buf, bool forever);
-  int getpkt_or_notif_sane (gdb::char_vector *buf, bool forever,
-			    bool *is_notif);
+  int getpkt (gdb::char_vector *buf, bool forever, bool *is_notif = nullptr);
   int remote_vkill (int pid);
   void remote_kill_k ();
 
@@ -8297,7 +8293,7 @@ remote_target::wait_ns (ptid_t ptid, struct target_waitstatus *status,
   /* If in non-stop mode, get out of getpkt even if a
      notification is received.	*/
 
-  ret = getpkt_or_notif_sane (&rs->buf, false /* forever */, &is_notif);
+  ret = getpkt (&rs->buf, false /* forever */, &is_notif);
   while (1)
     {
       if (ret != -1 && !is_notif)
@@ -8336,7 +8332,7 @@ remote_target::wait_ns (ptid_t ptid, struct target_waitstatus *status,
 	}
 
       /* Otherwise do a blocking wait.  */
-      ret = getpkt_or_notif_sane (&rs->buf, true /* forever */, &is_notif);
+      ret = getpkt (&rs->buf, true /* forever */, &is_notif);
     }
 }
 
@@ -8391,7 +8387,7 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
 	 However, before we do that we need to ensure that the caller
 	 knows how to take the target into/out of async mode.  */
       bool is_notif;
-      int ret = getpkt_or_notif_sane (&rs->buf, forever, &is_notif);
+      int ret = getpkt (&rs->buf, forever, &is_notif);
 
       /* GDB gets a notification.  Return to core as this event is
 	 not interesting.  */
@@ -10056,9 +10052,7 @@ show_watchdog (struct ui_file *file, int from_tty,
    packet).  */
 
 int
-remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
-				       bool forever,
-				       bool *is_notif)
+remote_target::getpkt (gdb::char_vector *buf, bool forever, bool *is_notif)
 {
   struct remote_state *rs = get_remote_state ();
   int c;
@@ -10195,25 +10189,6 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
     }
 }
 
-/* Read a packet from the remote machine, with error checking, and
-   store it in *BUF.  Resize *BUF if necessary to hold the result.  If
-   FOREVER, wait forever rather than timing out; this is used (in
-   synchronous mode) to wait for a target that is is executing user
-   code to stop.  */
-
-int
-remote_target::getpkt (gdb::char_vector *buf, bool forever)
-{
-  return getpkt_or_notif_sane_1 (buf, forever, NULL);
-}
-
-int
-remote_target::getpkt_or_notif_sane (gdb::char_vector *buf, bool forever,
-				     bool *is_notif)
-{
-  return getpkt_or_notif_sane_1 (buf, forever, is_notif);
-}
-
 /* Kill any new fork children of inferior INF that haven't been
    processed by follow_fork.  */
 

-- 
2.40.1

Re: [PATCH 0/4] Unify getpkt methods in remote.c

[John Baldwin]

On 8/28/23 10:14 AM, Tom Tromey via Gdb-patches wrote:
> This series unifies the various getpkt methods in remote.c, and
> applies a little bit of bool-ification as well.
> 
> ---
> Tom Tromey (4):
>        Remove getpkt_sane
>        Remove expecting_notif parameter from getpkt_or_notif_sane_1
>        Use bool in getpkt
>        Unify getpkt and getpkt_or_notif_sane
> 
>   gdb/remote.c | 242 +++++++++++++++++++++++++----------------------------------
>   1 file changed, 103 insertions(+), 139 deletions(-)
> ---
> base-commit: b8a175b415454df6a039ba0b5d2ff13c3c180275
> change-id: 20230828-getpkt-cleanup-126c625821bf
> 
> Best regards,

These all look fine to me.  My only thought was if you wanted to
make use of a default value for the 'forever' argument, or perhaps
have a 'getpkt_wait' wrapper.  My one worry about bool arguments
to functions (in general) is that true/false often do not convey
obvious meaning in context.  One way to deal with this can be to
use dedicated enums, e.g.:

enum getpkt_forever {
     NON_BLOCKING,
     BLOCKING
}

so that the invocations have more obvious intent.  However, reading
the patch 3 in particular, it seems like the vast majority of getpkt
calls end up passing false, so I wonder if you might not like to
end up with something like this at the end:

    int getpkt_1 (gdb::char_vector *buf, bool forever, bool *is_notif);
    int getpkt (gdb::char_vector *buf, bool *is_notif = nullptr)
    { return getpkt_1 (buf, false, is_notif); }
    int getpkt_wait (gdb::char_vector *buf, bool *is_notif = nullptr)
    { return getpkt_1 (buf, true, is_notif); }

Maybe you could get by with defaulting both arguments to getpkt and
not needing getpkt_1 at all, just having getpkt_wait() as wrapper?
I think that might end up being more readable as in many cases in
the resulting code you'd end up with:

    putpkt ("<mumble>");
    getpkt (&rs->buf);

(You could also call the wrapper `getpkt_forever` if that seems like
a better name.)

-- 
John Baldwin

Re: [PATCH 0/4] Unify getpkt methods in remote.c

[Tom Tromey]

>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

John>    int getpkt_1 (gdb::char_vector *buf, bool forever, bool *is_notif);
John>    int getpkt (gdb::char_vector *buf, bool *is_notif = nullptr)
John>    { return getpkt_1 (buf, false, is_notif); }
John>    int getpkt_wait (gdb::char_vector *buf, bool *is_notif = nullptr)
John>    { return getpkt_1 (buf, true, is_notif); }

John> Maybe you could get by with defaulting both arguments to getpkt and
John> not needing getpkt_1 at all, just having getpkt_wait() as wrapper?

Yeah, unless the full API is unwieldy, I tend to prefer default
arguments.

I can tack on a patch to add this for the 'forever' argument.

Tom