* [PATCH v2 1/2] remote.c: Use packet_check_result
@ 2024-03-19 13:58 Alexandra Hájková
2024-03-19 13:58 ` [PATCH v2 2/2] remote.c: Make packet_ok return struct packet_result Alexandra Hájková
2024-03-25 10:21 ` [PATCH v2 1/2] remote.c: Use packet_check_result Andrew Burgess
0 siblings, 2 replies; 4+ messages in thread
From: Alexandra Hájková @ 2024-03-19 13:58 UTC (permalink / raw)
To: gdb-patches; +Cc: ahajkova
From: Alexandra Hájková <ahajkova@redhat.com>
when processing the GDBserver reply to qRcmd packet.
Print error message or the error code.
Currently, when qRcmd request returns an error,
GDB just prints:
Protocol error with Rcmd
After this change, GDB will also print the error code:
Protocol error with Rcmd: 01.
Add an accept_msg argument to packet_check result. qRcmd
request (such as many other packets) does not recognise
"E.msg" form as an error right now. We want to recognise
"E.msg" as an error response only for the packets where
it's documented.
---
gdb/remote.c | 75 +++++++++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 33 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 14c8b020b1e..8462b7e4e60 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2451,11 +2451,15 @@ add_packet_config_cmd (const unsigned int which_packet, const char *name,
}
}
-/* Check GDBserver's reply packet. Return packet_result
- structure which contains the packet_status enum
- and an error message for the PACKET_ERROR case. */
+/* Check GDBserver's reply packet. Return packet_result structure
+ which contains the packet_status enum and an error message for the
+ PACKET_ERROR case.
+
+ An error packet can always take the form Exx (where xx is a hex
+ code). When ACCEPT_MSG is true error messages can also take the
+ form E.msg (where msg is any arbitrary string). */
static packet_result
-packet_check_result (const char *buf)
+packet_check_result (const char *buf, bool accept_msg)
{
if (buf[0] != '\0')
{
@@ -2467,14 +2471,20 @@ packet_check_result (const char *buf)
/* "Enn" - definitely an error. */
return { PACKET_ERROR, buf + 1 };
- /* Always treat "E." as an error. This will be used for
- more verbose error messages, such as E.memtypes. */
- if (buf[0] == 'E' && buf[1] == '.')
+ /* Not every request accepts an error in a E.msg form.
+ Some packets accepts only Enn, in this case E. is not
+ defined and E. is treated as PACKET_OK. */
+ if (accept_msg)
{
- if (buf[2] != '\0')
- return { PACKET_ERROR, buf + 2 };
- else
- return { PACKET_ERROR, "no error provided" };
+ /* Always treat "E." as an error. This will be used for
+ more verbose error messages, such as E.memtypes. */
+ if (buf[0] == 'E' && buf[1] == '.')
+ {
+ if (buf[2] != '\0')
+ return { PACKET_ERROR, buf + 2 };
+ else
+ return { PACKET_ERROR, "no error provided" };
+ }
}
/* The packet may or may not be OK. Just assume it is. */
@@ -2488,9 +2498,9 @@ packet_check_result (const char *buf)
}
static packet_result
-packet_check_result (const gdb::char_vector &buf)
+packet_check_result (const gdb::char_vector &buf, bool accept_msg)
{
- return packet_check_result (buf.data ());
+ return packet_check_result (buf.data (), accept_msg);
}
packet_status
@@ -2503,7 +2513,7 @@ remote_features::packet_ok (const char *buf, const int which_packet)
&& config->support == PACKET_DISABLE)
internal_error (_("packet_ok: attempt to use a disabled packet"));
- packet_result result = packet_check_result (buf);
+ packet_result result = packet_check_result (buf, true);
switch (result.status ())
{
case PACKET_OK:
@@ -8831,7 +8841,7 @@ remote_target::send_g_packet ()
xsnprintf (rs->buf.data (), get_remote_packet_size (), "g");
putpkt (rs->buf);
getpkt (&rs->buf);
- packet_result result = packet_check_result (rs->buf);
+ packet_result result = packet_check_result (rs->buf, true);
if (result.status () == PACKET_ERROR)
error (_("Could not read registers; remote failure reply '%s'"),
result.err_msg ());
@@ -9140,7 +9150,7 @@ remote_target::store_registers_using_G (const struct regcache *regcache)
bin2hex (regs, p, rsa->sizeof_g_packet);
putpkt (rs->buf);
getpkt (&rs->buf);
- packet_result pkt_status = packet_check_result (rs->buf);
+ packet_result pkt_status = packet_check_result (rs->buf, true);
if (pkt_status.status () == PACKET_ERROR)
error (_("Could not write registers; remote failure reply '%s'"),
pkt_status.err_msg ());
@@ -9748,7 +9758,7 @@ remote_target::remote_send_printf (const char *format, ...)
rs->buf[0] = '\0';
getpkt (&rs->buf);
- return packet_check_result (rs->buf).status ();
+ return packet_check_result (rs->buf, true).status ();
}
/* Flash writing can take quite some time. We'll set
@@ -11931,20 +11941,19 @@ remote_target::rcmd (const char *command, struct ui_file *outbuf)
continue;
}
buf = rs->buf.data ();
- if (buf[0] == '\0')
- error (_("Target does not support this command."));
if (buf[0] == 'O' && buf[1] != 'K')
{
remote_console_output (buf + 1); /* 'O' message from stub. */
continue;
}
- if (strcmp (buf, "OK") == 0)
+ packet_result result = packet_check_result (buf, false);
+ if (result.status () == PACKET_OK)
break;
- if (strlen (buf) == 3 && buf[0] == 'E'
- && isxdigit (buf[1]) && isxdigit (buf[2]))
- {
- error (_("Protocol error with Rcmd"));
- }
+ else if (result.status () == PACKET_UNKNOWN)
+ error (_("Target does not support this command."));
+ else
+ error (_("Protocol error with Rcmd: %s."), result.err_msg ());
+
for (p = buf; p[0] != '\0' && p[1] != '\0'; p += 2)
{
char c = (fromhex (p[0]) << 4) + fromhex (p[1]);
@@ -15571,7 +15580,7 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
getpkt (&rs->buf);
/* Verify if the request was successful. */
- return packet_check_result (rs->buf).status () == PACKET_OK;
+ return (packet_check_result (rs->buf, true).status () == PACKET_OK);
}
/* Return true if remote target T is non-stop. */
@@ -15672,26 +15681,26 @@ static void
test_packet_check_result ()
{
std::string buf = "E.msg";
- packet_result result = packet_check_result (buf.data ());
+ packet_result result = packet_check_result (buf.data (), true);
SELF_CHECK (result.status () == PACKET_ERROR);
SELF_CHECK (strcmp(result.err_msg (), "msg") == 0);
- result = packet_check_result ("E01");
+ result = packet_check_result ("E01", true);
SELF_CHECK (result.status () == PACKET_ERROR);
SELF_CHECK (strcmp(result.err_msg (), "01") == 0);
- SELF_CHECK (packet_check_result ("E1").status () == PACKET_OK);
+ SELF_CHECK (packet_check_result ("E1", true).status () == PACKET_OK);
- SELF_CHECK (packet_check_result ("E000").status () == PACKET_OK);
+ SELF_CHECK (packet_check_result ("E000", true).status () == PACKET_OK);
- result = packet_check_result ("E.");
+ result = packet_check_result ("E.", true);
SELF_CHECK (result.status () == PACKET_ERROR);
SELF_CHECK (strcmp(result.err_msg (), "no error provided") == 0);
- SELF_CHECK (packet_check_result ("some response").status () == PACKET_OK);
+ SELF_CHECK (packet_check_result ("some response", true).status () == PACKET_OK);
- SELF_CHECK (packet_check_result ("").status () == PACKET_UNKNOWN);
+ SELF_CHECK (packet_check_result ("", true).status () == PACKET_UNKNOWN);
}
} // namespace selftests
#endif /* GDB_SELF_TEST */
--
2.44.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] remote.c: Make packet_ok return struct packet_result
2024-03-19 13:58 [PATCH v2 1/2] remote.c: Use packet_check_result Alexandra Hájková
@ 2024-03-19 13:58 ` Alexandra Hájková
2024-03-26 9:48 ` Andrew Burgess
2024-03-25 10:21 ` [PATCH v2 1/2] remote.c: Use packet_check_result Andrew Burgess
1 sibling, 1 reply; 4+ messages in thread
From: Alexandra Hájková @ 2024-03-19 13:58 UTC (permalink / raw)
To: gdb-patches; +Cc: ahajkova
From: Alexandra Hájková <ahajkova@redhat.com>
This allows to print the error message stored in a packet_result
to be easily used in the calling function.
---
gdb/remote.c | 199 +++++++++++++++++++++++++--------------------------
1 file changed, 96 insertions(+), 103 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 8462b7e4e60..95e193ba543 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -765,8 +765,8 @@ struct remote_features
/* Check result value in BUF for packet WHICH_PACKET and update the packet's
support configuration accordingly. */
- packet_status packet_ok (const char *buf, const int which_packet);
- packet_status packet_ok (const gdb::char_vector &buf, const int which_packet);
+ packet_result packet_ok (const char *buf, const int which_packet);
+ packet_result packet_ok (const gdb::char_vector &buf, const int which_packet);
/* Configuration of a remote target's memory read packet. */
memory_packet_config m_memory_read_packet_config;
@@ -2503,7 +2503,7 @@ packet_check_result (const gdb::char_vector &buf, bool accept_msg)
return packet_check_result (buf.data (), accept_msg);
}
-packet_status
+packet_result
remote_features::packet_ok (const char *buf, const int which_packet)
{
packet_config *config = &m_protocol_packets[which_packet];
@@ -2549,10 +2549,10 @@ remote_features::packet_ok (const char *buf, const int which_packet)
break;
}
- return result.status ();
+ return result;
}
-packet_status
+packet_result
remote_features::packet_ok (const gdb::char_vector &buf, const int which_packet)
{
return packet_ok (buf.data (), which_packet);
@@ -2739,14 +2739,15 @@ remote_target::remote_query_attached (int pid)
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_qAttached))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_qAttached);
+ switch (result.status())
{
case PACKET_OK:
if (strcmp (rs->buf.data (), "1") == 0)
return 1;
break;
case PACKET_ERROR:
- warning (_("Remote failure reply: %s"), rs->buf.data ());
+ warning (_("Remote failure reply: %s"), result.err_msg());
break;
case PACKET_UNKNOWN:
break;
@@ -3051,7 +3052,6 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
gdb::array_view<const int> syscall_counts)
{
const char *catch_packet;
- enum packet_status result;
int n_sysno = 0;
if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
@@ -3107,8 +3107,8 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
putpkt (catch_packet);
getpkt (&rs->buf);
- result = m_features.packet_ok (rs->buf, PACKET_QCatchSyscalls);
- if (result == PACKET_OK)
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_QCatchSyscalls);
+ if (result.status() == PACKET_OK)
return 0;
else
return -1;
@@ -5113,7 +5113,8 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
{
putpkt ("QStartNoAckMode");
getpkt (&rs->buf);
- if (m_features.packet_ok (rs->buf, PACKET_QStartNoAckMode) == PACKET_OK)
+ if ((m_features.packet_ok (rs->buf, PACKET_QStartNoAckMode)).status ()
+ == PACKET_OK)
rs->noack_mode = 1;
}
@@ -5898,9 +5899,10 @@ remote_target::remote_query_supported ()
/* If an error occurred, warn, but do not return - just reset the
buffer to empty and go on to disable features. */
- if (m_features.packet_ok (rs->buf, PACKET_qSupported) == PACKET_ERROR)
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_qSupported);
+ if (result.status () == PACKET_ERROR)
{
- warning (_("Remote failure reply: %s"), rs->buf.data ());
+ warning (_("Remote failure reply: %s"), result.err_msg ());
rs->buf[0] = 0;
}
}
@@ -6552,7 +6554,8 @@ extended_remote_target::attach (const char *args, int from_tty)
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_vAttach))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_vAttach);
+ switch (result.status ())
{
case PACKET_OK:
if (!target_is_non_stop_p ())
@@ -6568,9 +6571,9 @@ extended_remote_target::attach (const char *args, int from_tty)
break;
case PACKET_UNKNOWN:
error (_("This target does not support attaching to a process"));
- default:
- error (_("Attaching to %s failed"),
- target_pid_to_str (ptid_t (pid)).c_str ());
+ case PACKET_ERROR:
+ error (_("Attaching to %s failed: %s"),
+ target_pid_to_str (ptid_t (pid)).c_str (), result.err_msg ());
}
switch_to_inferior_no_thread (remote_add_inferior (false, pid, 1, 0));
@@ -7493,14 +7496,15 @@ remote_target::remote_interrupt_ns ()
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_vCtrlC))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_vCtrlC);
+ switch (result.status ())
{
case PACKET_OK:
break;
case PACKET_UNKNOWN:
error (_("No support for interrupting the remote target."));
case PACKET_ERROR:
- error (_("Interrupting target failed: %s"), rs->buf.data ());
+ error (_("Interrupting target failed: %s"), result.err_msg ());
}
}
@@ -8796,7 +8800,8 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
buf = rs->buf.data ();
- switch (m_features.packet_ok (rs->buf, PACKET_p))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_p);
+ switch (result.status ())
{
case PACKET_OK:
break;
@@ -8805,7 +8810,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
case PACKET_ERROR:
error (_("Could not fetch register \"%s\"; remote failure reply '%s'"),
gdbarch_register_name (regcache->arch (), reg->regnum),
- buf);
+ result.err_msg ());
}
/* If this register is unfetchable, tell the regcache. */
@@ -9102,13 +9107,14 @@ remote_target::store_register_using_P (const struct regcache *regcache,
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_P))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_P);
+ switch (result.status ())
{
case PACKET_OK:
return 1;
case PACKET_ERROR:
error (_("Could not write register \"%s\"; remote failure reply '%s'"),
- gdbarch_register_name (gdbarch, reg->regnum), rs->buf.data ());
+ gdbarch_register_name (gdbarch, reg->regnum), result.err_msg ());
case PACKET_UNKNOWN:
return 0;
default:
@@ -10539,7 +10545,7 @@ remote_target::remote_vkill (int pid)
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_vKill))
+ switch ((m_features.packet_ok (rs->buf, PACKET_vKill)).status ())
{
case PACKET_OK:
return 0;
@@ -10695,7 +10701,7 @@ remote_target::extended_remote_run (const std::string &args)
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_vRun))
+ switch ((m_features.packet_ok (rs->buf, PACKET_vRun)).status ())
{
case PACKET_OK:
/* We have a wait response. All is well. */
@@ -10802,11 +10808,14 @@ remote_target::extended_remote_set_inferior_cwd ()
putpkt (rs->buf);
getpkt (&rs->buf);
- if (m_features.packet_ok (rs->buf, PACKET_QSetWorkingDir) != PACKET_OK)
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_QSetWorkingDir);
+ if (result.status () == PACKET_ERROR)
error (_("\
Remote replied unexpectedly while setting the inferior's working\n\
directory: %s"),
- rs->buf.data ());
+ result.err_msg ());
+ if (result.status () == PACKET_UNKNOWN)
+ error (_("Remote target failed to process setting the inferior's working directory"));
}
}
@@ -10975,7 +10984,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_Z0))
+ switch ((m_features.packet_ok (rs->buf, PACKET_Z0)).status ())
{
case PACKET_ERROR:
return -1;
@@ -11076,8 +11085,8 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
- + to_underlying (packet))))
+ switch ((m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
+ + to_underlying (packet)))).status ())
{
case PACKET_ERROR:
return -1;
@@ -11125,8 +11134,8 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
- + to_underlying (packet))))
+ switch ((m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
+ + to_underlying (packet)))).status ())
{
case PACKET_ERROR:
case PACKET_UNKNOWN:
@@ -11257,7 +11266,6 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
CORE_ADDR addr = bp_tgt->reqstd_address;
struct remote_state *rs;
char *p, *endbuf;
- char *message;
if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
return -1;
@@ -11288,16 +11296,11 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_Z1))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_Z1);
+ switch (result.status ())
{
case PACKET_ERROR:
- if (rs->buf[1] == '.')
- {
- message = strchr (&rs->buf[2], '.');
- if (message)
- error (_("Remote failure reply: %s"), message + 1);
- }
- return -1;
+ error (_("Remote failure reply: %s"), result.err_msg ());
case PACKET_UNKNOWN:
return -1;
case PACKET_OK:
@@ -11335,7 +11338,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_Z1))
+ switch ((m_features.packet_ok (rs->buf, PACKET_Z1)).status ())
{
case PACKET_ERROR:
case PACKET_UNKNOWN:
@@ -11376,7 +11379,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
getpkt (&rs->buf);
- status = m_features.packet_ok (rs->buf, PACKET_qCRC);
+ status = (m_features.packet_ok (rs->buf, PACKET_qCRC)).status ();
if (status == PACKET_ERROR)
return -1;
else if (status == PACKET_OK)
@@ -11498,7 +11501,7 @@ remote_target::remote_write_qxfer (const char *object_name,
if (putpkt_binary (rs->buf.data (), i + buf_len) < 0
|| getpkt (&rs->buf) < 0
- || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
+ || (m_features.packet_ok (rs->buf, which_packet)).status () != PACKET_OK)
return TARGET_XFER_E_IO;
unpack_varlen_hex (rs->buf.data (), &n);
@@ -11563,7 +11566,7 @@ remote_target::remote_read_qxfer (const char *object_name,
rs->buf[0] = '\0';
packet_len = getpkt (&rs->buf);
if (packet_len < 0
- || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
+ || (m_features.packet_ok (rs->buf, which_packet)).status () != PACKET_OK)
return TARGET_XFER_E_IO;
if (rs->buf[0] != 'l' && rs->buf[0] != 'm')
@@ -11868,7 +11871,8 @@ remote_target::search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
if (putpkt_binary (rs->buf.data (), i + escaped_pattern_len) < 0
|| getpkt (&rs->buf) < 0
- || m_features.packet_ok (rs->buf, PACKET_qSearch_memory) != PACKET_OK)
+ || (m_features.packet_ok (rs->buf, PACKET_qSearch_memory)).status ()
+ != PACKET_OK)
{
/* The request may not have worked because the command is not
supported. If so, fall back to the simple way. */
@@ -12261,7 +12265,6 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
struct remote_state *rs = get_remote_state ();
char *p = rs->buf.data ();
char *endp = p + get_remote_packet_size ();
- enum packet_status result;
strcpy (p, "qGetTLSAddr:");
p += strlen (p);
@@ -12274,15 +12277,15 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
putpkt (rs->buf);
getpkt (&rs->buf);
- result = m_features.packet_ok (rs->buf, PACKET_qGetTLSAddr);
- if (result == PACKET_OK)
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_qGetTLSAddr);
+ if (result.status () == PACKET_OK)
{
ULONGEST addr;
unpack_varlen_hex (rs->buf.data (), &addr);
return addr;
}
- else if (result == PACKET_UNKNOWN)
+ else if (result.status () == PACKET_UNKNOWN)
throw_error (TLS_GENERIC_ERROR,
_("Remote target doesn't support qGetTLSAddr packet"));
else
@@ -12307,7 +12310,6 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
struct remote_state *rs = get_remote_state ();
char *p = rs->buf.data ();
char *endp = p + get_remote_packet_size ();
- enum packet_status result;
strcpy (p, "qGetTIBAddr:");
p += strlen (p);
@@ -12316,8 +12318,8 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
putpkt (rs->buf);
getpkt (&rs->buf);
- result = m_features.packet_ok (rs->buf, PACKET_qGetTIBAddr);
- if (result == PACKET_OK)
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_qGetTIBAddr);
+ if (result.status () == PACKET_OK)
{
ULONGEST val;
unpack_varlen_hex (rs->buf.data (), &val);
@@ -12325,10 +12327,11 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
*addr = (CORE_ADDR) val;
return true;
}
- else if (result == PACKET_UNKNOWN)
+ else if (result.status () == PACKET_UNKNOWN)
error (_("Remote target doesn't support qGetTIBAddr packet"));
else
- error (_("Remote target failed to process qGetTIBAddr request"));
+ error (_("Remote target failed to process qGetTIBAddr request, %s"),
+ result.err_msg ());
}
else
error (_("qGetTIBAddr not supported or disabled on this target"));
@@ -12584,7 +12587,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
return -1;
}
- switch (m_features.packet_ok (rs->buf, which_packet))
+ switch ((m_features.packet_ok (rs->buf, which_packet)).status ())
{
case PACKET_ERROR:
*remote_errno = FILEIO_EINVAL;
@@ -13872,7 +13875,6 @@ remote_target::get_trace_status (struct trace_status *ts)
{
/* Initialize it just to avoid a GCC false warning. */
char *p = NULL;
- enum packet_status result;
struct remote_state *rs = get_remote_state ();
if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE)
@@ -13898,11 +13900,16 @@ remote_target::get_trace_status (struct trace_status *ts)
throw;
}
- result = m_features.packet_ok (p, PACKET_qTStatus);
+ packet_result result = m_features.packet_ok (p, PACKET_qTStatus);
- /* If the remote target doesn't do tracing, flag it. */
- if (result == PACKET_UNKNOWN)
- return -1;
+ switch (result.status ())
+ {
+ case PACKET_ERROR:
+ error (_("Remote failure reply: %s"), result.err_msg ());
+ /* If the remote target doesn't do tracing, flag it. */
+ case PACKET_UNKNOWN:
+ return -1;
+ }
/* We're working with a live target. */
ts->filename = NULL;
@@ -14252,7 +14259,6 @@ remote_target::set_trace_buffer_size (LONGEST val)
struct remote_state *rs = get_remote_state ();
char *buf = rs->buf.data ();
char *endbuf = buf + get_remote_packet_size ();
- enum packet_status result;
gdb_assert (val >= 0 || val == -1);
buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
@@ -14267,10 +14273,15 @@ remote_target::set_trace_buffer_size (LONGEST val)
putpkt (rs->buf);
remote_get_noisy_reply ();
- result = m_features.packet_ok (rs->buf, PACKET_QTBuffer_size);
-
- if (result != PACKET_OK)
- warning (_("Bogus reply from target: %s"), rs->buf.data ());
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_QTBuffer_size);
+ switch (result.status ())
+ {
+ case PACKET_ERROR:
+ warning (_("Bogus reply from target: %s"), result.err_msg ());
+ break;
+ case PACKET_UNKNOWN:
+ warning (_("Remote target failed to process the request "));
+ }
}
}
@@ -14696,14 +14707,9 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
putpkt (buf);
getpkt (&rs->buf);
- if (m_features.packet_ok (buf, PACKET_Qbtrace_conf_bts_size)
- == PACKET_ERROR)
- {
- if (buf[0] == 'E' && buf[1] == '.')
- error (_("Failed to configure the BTS buffer size: %s"), buf + 2);
- else
- error (_("Failed to configure the BTS buffer size."));
- }
+ packet_result result = m_features.packet_ok (buf, PACKET_Qbtrace_conf_bts_size);
+ if (result.status () == PACKET_ERROR)
+ error (_("Failed to configure the BTS buffer size: %s"), result.err_msg ());
rs->btrace_config.bts.size = conf->bts.size;
}
@@ -14719,14 +14725,9 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
putpkt (buf);
getpkt (&rs->buf);
- if (m_features.packet_ok (buf, PACKET_Qbtrace_conf_pt_size)
- == PACKET_ERROR)
- {
- if (buf[0] == 'E' && buf[1] == '.')
- error (_("Failed to configure the trace buffer size: %s"), buf + 2);
- else
- error (_("Failed to configure the trace buffer size."));
- }
+ packet_result result = m_features.packet_ok (buf, PACKET_Qbtrace_conf_pt_size);
+ if (result.status () == PACKET_ERROR)
+ error (_("Failed to configure the trace buffer size: %s"), result.err_msg ());
rs->btrace_config.pt.size = conf->pt.size;
}
@@ -14841,15 +14842,10 @@ remote_target::enable_btrace (thread_info *tp,
putpkt (rs->buf);
getpkt (&rs->buf);
- if (m_features.packet_ok (rs->buf, which_packet) == PACKET_ERROR)
- {
- if (rs->buf[0] == 'E' && rs->buf[1] == '.')
- error (_("Could not enable branch tracing for %s: %s"),
- target_pid_to_str (ptid).c_str (), &rs->buf[2]);
- else
- error (_("Could not enable branch tracing for %s."),
- target_pid_to_str (ptid).c_str ());
- }
+ packet_result result = m_features.packet_ok (rs->buf, which_packet);
+ if (result.status () == PACKET_ERROR)
+ error (_("Could not enable branch tracing for %s: %s"),
+ target_pid_to_str (ptid).c_str (), result.err_msg ());
btrace_target_info *tinfo = new btrace_target_info { ptid };
@@ -14887,15 +14883,10 @@ remote_target::disable_btrace (struct btrace_target_info *tinfo)
putpkt (rs->buf);
getpkt (&rs->buf);
- if (m_features.packet_ok (rs->buf, PACKET_Qbtrace_off) == PACKET_ERROR)
- {
- if (rs->buf[0] == 'E' && rs->buf[1] == '.')
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_Qbtrace_off);
+ if (result.status () == PACKET_ERROR)
error (_("Could not disable branch tracing for %s: %s"),
- target_pid_to_str (tinfo->ptid).c_str (), &rs->buf[2]);
- else
- error (_("Could not disable branch tracing for %s."),
- target_pid_to_str (tinfo->ptid).c_str ());
- }
+ target_pid_to_str (tinfo->ptid).c_str (), result.err_msg ());
delete tinfo;
}
@@ -15160,7 +15151,8 @@ remote_target::thread_events (int enable)
putpkt (rs->buf);
getpkt (&rs->buf);
- switch (m_features.packet_ok (rs->buf, PACKET_QThreadEvents))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_QThreadEvents);
+ switch (result.status ())
{
case PACKET_OK:
if (strcmp (rs->buf.data (), "OK") != 0)
@@ -15168,7 +15160,7 @@ remote_target::thread_events (int enable)
rs->last_thread_events = enable;
break;
case PACKET_ERROR:
- warning (_("Remote failure reply: %s"), rs->buf.data ());
+ warning (_("Remote failure reply: %s"), result.err_msg ());
break;
case PACKET_UNKNOWN:
break;
@@ -15215,14 +15207,15 @@ remote_target::commit_requested_thread_options ()
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (m_features.packet_ok (rs->buf, PACKET_QThreadOptions))
+ packet_result result = m_features.packet_ok (rs->buf, PACKET_QThreadOptions);
+ switch (result.status ())
{
case PACKET_OK:
if (strcmp (rs->buf.data (), "OK") != 0)
error (_("Remote refused setting thread options: %s"), rs->buf.data ());
break;
case PACKET_ERROR:
- error (_("Remote failure reply: %s"), rs->buf.data ());
+ error (_("Remote failure reply: %s"), result.err_msg ());
case PACKET_UNKNOWN:
gdb_assert_not_reached ("PACKET_UNKNOWN");
break;
--
2.44.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/2] remote.c: Use packet_check_result
2024-03-19 13:58 [PATCH v2 1/2] remote.c: Use packet_check_result Alexandra Hájková
2024-03-19 13:58 ` [PATCH v2 2/2] remote.c: Make packet_ok return struct packet_result Alexandra Hájková
@ 2024-03-25 10:21 ` Andrew Burgess
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Burgess @ 2024-03-25 10:21 UTC (permalink / raw)
To: Alexandra Hájková, gdb-patches; +Cc: ahajkova
Alexandra Hájková <ahajkova@khirnov.net> writes:
> From: Alexandra Hájková <ahajkova@redhat.com>
>
> when processing the GDBserver reply to qRcmd packet.
> Print error message or the error code.
> Currently, when qRcmd request returns an error,
> GDB just prints:
>
> Protocol error with Rcmd
>
> After this change, GDB will also print the error code:
>
> Protocol error with Rcmd: 01.
>
> Add an accept_msg argument to packet_check result. qRcmd
> request (such as many other packets) does not recognise
> "E.msg" form as an error right now. We want to recognise
> "E.msg" as an error response only for the packets where
> it's documented.
> ---
> gdb/remote.c | 75 +++++++++++++++++++++++++++++-----------------------
> 1 file changed, 42 insertions(+), 33 deletions(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 14c8b020b1e..8462b7e4e60 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -2451,11 +2451,15 @@ add_packet_config_cmd (const unsigned int which_packet, const char *name,
> }
> }
>
> -/* Check GDBserver's reply packet. Return packet_result
> - structure which contains the packet_status enum
> - and an error message for the PACKET_ERROR case. */
> +/* Check GDBserver's reply packet. Return packet_result structure
> + which contains the packet_status enum and an error message for the
> + PACKET_ERROR case.
> +
> + An error packet can always take the form Exx (where xx is a hex
> + code). When ACCEPT_MSG is true error messages can also take the
> + form E.msg (where msg is any arbitrary string). */
> static packet_result
> -packet_check_result (const char *buf)
> +packet_check_result (const char *buf, bool accept_msg)
> {
> if (buf[0] != '\0')
> {
> @@ -2467,14 +2471,20 @@ packet_check_result (const char *buf)
> /* "Enn" - definitely an error. */
> return { PACKET_ERROR, buf + 1 };
>
> - /* Always treat "E." as an error. This will be used for
> - more verbose error messages, such as E.memtypes. */
> - if (buf[0] == 'E' && buf[1] == '.')
> + /* Not every request accepts an error in a E.msg form.
> + Some packets accepts only Enn, in this case E. is not
> + defined and E. is treated as PACKET_OK. */
> + if (accept_msg)
> {
> - if (buf[2] != '\0')
> - return { PACKET_ERROR, buf + 2 };
> - else
> - return { PACKET_ERROR, "no error provided" };
> + /* Always treat "E." as an error. This will be used for
> + more verbose error messages, such as E.memtypes. */
> + if (buf[0] == 'E' && buf[1] == '.')
> + {
> + if (buf[2] != '\0')
> + return { PACKET_ERROR, buf + 2 };
> + else
> + return { PACKET_ERROR, "no error provided" };
> + }
> }
>
> /* The packet may or may not be OK. Just assume it is. */
> @@ -2488,9 +2498,9 @@ packet_check_result (const char *buf)
> }
>
> static packet_result
> -packet_check_result (const gdb::char_vector &buf)
> +packet_check_result (const gdb::char_vector &buf, bool accept_msg)
> {
> - return packet_check_result (buf.data ());
> + return packet_check_result (buf.data (), accept_msg);
> }
>
> packet_status
> @@ -2503,7 +2513,7 @@ remote_features::packet_ok (const char *buf, const int which_packet)
> && config->support == PACKET_DISABLE)
> internal_error (_("packet_ok: attempt to use a disabled packet"));
>
> - packet_result result = packet_check_result (buf);
> + packet_result result = packet_check_result (buf, true);
> switch (result.status ())
> {
> case PACKET_OK:
> @@ -8831,7 +8841,7 @@ remote_target::send_g_packet ()
> xsnprintf (rs->buf.data (), get_remote_packet_size (), "g");
> putpkt (rs->buf);
> getpkt (&rs->buf);
> - packet_result result = packet_check_result (rs->buf);
> + packet_result result = packet_check_result (rs->buf, true);
> if (result.status () == PACKET_ERROR)
> error (_("Could not read registers; remote failure reply '%s'"),
> result.err_msg ());
> @@ -9140,7 +9150,7 @@ remote_target::store_registers_using_G (const struct regcache *regcache)
> bin2hex (regs, p, rsa->sizeof_g_packet);
> putpkt (rs->buf);
> getpkt (&rs->buf);
> - packet_result pkt_status = packet_check_result (rs->buf);
> + packet_result pkt_status = packet_check_result (rs->buf, true);
> if (pkt_status.status () == PACKET_ERROR)
> error (_("Could not write registers; remote failure reply '%s'"),
> pkt_status.err_msg ());
> @@ -9748,7 +9758,7 @@ remote_target::remote_send_printf (const char *format, ...)
> rs->buf[0] = '\0';
> getpkt (&rs->buf);
>
> - return packet_check_result (rs->buf).status ();
> + return packet_check_result (rs->buf, true).status ();
> }
>
> /* Flash writing can take quite some time. We'll set
> @@ -11931,20 +11941,19 @@ remote_target::rcmd (const char *command, struct ui_file *outbuf)
> continue;
> }
> buf = rs->buf.data ();
> - if (buf[0] == '\0')
> - error (_("Target does not support this command."));
> if (buf[0] == 'O' && buf[1] != 'K')
> {
> remote_console_output (buf + 1); /* 'O' message from stub. */
> continue;
> }
> - if (strcmp (buf, "OK") == 0)
> + packet_result result = packet_check_result (buf, false);
> + if (result.status () == PACKET_OK)
> break;
Unfortunately this is a change in behaviour. PACKET_OK status does not
mean that the packet contains the text "OK", it just means that the
packet was not an error, and was not the empty (PACKET_UNKNOWN) packet.
If the packet contained the text "ABCD" then previously we would end up
in the loop below which converts the value from a hex-encoded string,
but now I believe we'll end up breaking out of the while loop.
> - if (strlen (buf) == 3 && buf[0] == 'E'
> - && isxdigit (buf[1]) && isxdigit (buf[2]))
> - {
> - error (_("Protocol error with Rcmd"));
> - }
> + else if (result.status () == PACKET_UNKNOWN)
> + error (_("Target does not support this command."));
> + else
> + error (_("Protocol error with Rcmd: %s."), result.err_msg ());
> +
> for (p = buf; p[0] != '\0' && p[1] != '\0'; p += 2)
> {
> char c = (fromhex (p[0]) << 4) + fromhex (p[1]);
> @@ -15571,7 +15580,7 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
> getpkt (&rs->buf);
>
> /* Verify if the request was successful. */
> - return packet_check_result (rs->buf).status () == PACKET_OK;
> + return (packet_check_result (rs->buf, true).status () == PACKET_OK);
> }
>
> /* Return true if remote target T is non-stop. */
> @@ -15672,26 +15681,26 @@ static void
> test_packet_check_result ()
> {
> std::string buf = "E.msg";
> - packet_result result = packet_check_result (buf.data ());
> + packet_result result = packet_check_result (buf.data (), true);
>
> SELF_CHECK (result.status () == PACKET_ERROR);
> SELF_CHECK (strcmp(result.err_msg (), "msg") == 0);
>
> - result = packet_check_result ("E01");
> + result = packet_check_result ("E01", true);
> SELF_CHECK (result.status () == PACKET_ERROR);
> SELF_CHECK (strcmp(result.err_msg (), "01") == 0);
>
> - SELF_CHECK (packet_check_result ("E1").status () == PACKET_OK);
> + SELF_CHECK (packet_check_result ("E1", true).status () == PACKET_OK);
>
> - SELF_CHECK (packet_check_result ("E000").status () == PACKET_OK);
> + SELF_CHECK (packet_check_result ("E000", true).status () == PACKET_OK);
>
> - result = packet_check_result ("E.");
> + result = packet_check_result ("E.", true);
> SELF_CHECK (result.status () == PACKET_ERROR);
> SELF_CHECK (strcmp(result.err_msg (), "no error provided") == 0);
>
> - SELF_CHECK (packet_check_result ("some response").status () == PACKET_OK);
> + SELF_CHECK (packet_check_result ("some response", true).status () == PACKET_OK);
>
> - SELF_CHECK (packet_check_result ("").status () == PACKET_UNKNOWN);
> + SELF_CHECK (packet_check_result ("", true).status () == PACKET_UNKNOWN);
I wonder if we should add a unit test here where we pass an 'E.msg' and
pass the second argument to packet_check_result as false and ensure we
get back the results we expect (which I think is PACKET_OK).
Thanks,
Andrew
> }
> } // namespace selftests
> #endif /* GDB_SELF_TEST */
> --
> 2.44.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] remote.c: Make packet_ok return struct packet_result
2024-03-19 13:58 ` [PATCH v2 2/2] remote.c: Make packet_ok return struct packet_result Alexandra Hájková
@ 2024-03-26 9:48 ` Andrew Burgess
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Burgess @ 2024-03-26 9:48 UTC (permalink / raw)
To: Alexandra Hájková, gdb-patches; +Cc: ahajkova
Alexandra Hájková <ahajkova@khirnov.net> writes:
> From: Alexandra Hájková <ahajkova@redhat.com>
>
> This allows to print the error message stored in a packet_result
> to be easily used in the calling function.
This looks good.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
> ---
> gdb/remote.c | 199 +++++++++++++++++++++++++--------------------------
> 1 file changed, 96 insertions(+), 103 deletions(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 8462b7e4e60..95e193ba543 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -765,8 +765,8 @@ struct remote_features
>
> /* Check result value in BUF for packet WHICH_PACKET and update the packet's
> support configuration accordingly. */
> - packet_status packet_ok (const char *buf, const int which_packet);
> - packet_status packet_ok (const gdb::char_vector &buf, const int which_packet);
> + packet_result packet_ok (const char *buf, const int which_packet);
> + packet_result packet_ok (const gdb::char_vector &buf, const int which_packet);
>
> /* Configuration of a remote target's memory read packet. */
> memory_packet_config m_memory_read_packet_config;
> @@ -2503,7 +2503,7 @@ packet_check_result (const gdb::char_vector &buf, bool accept_msg)
> return packet_check_result (buf.data (), accept_msg);
> }
>
> -packet_status
> +packet_result
> remote_features::packet_ok (const char *buf, const int which_packet)
> {
> packet_config *config = &m_protocol_packets[which_packet];
> @@ -2549,10 +2549,10 @@ remote_features::packet_ok (const char *buf, const int which_packet)
> break;
> }
>
> - return result.status ();
> + return result;
> }
>
> -packet_status
> +packet_result
> remote_features::packet_ok (const gdb::char_vector &buf, const int which_packet)
> {
> return packet_ok (buf.data (), which_packet);
> @@ -2739,14 +2739,15 @@ remote_target::remote_query_attached (int pid)
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_qAttached))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_qAttached);
> + switch (result.status())
> {
> case PACKET_OK:
> if (strcmp (rs->buf.data (), "1") == 0)
> return 1;
> break;
> case PACKET_ERROR:
> - warning (_("Remote failure reply: %s"), rs->buf.data ());
> + warning (_("Remote failure reply: %s"), result.err_msg());
> break;
> case PACKET_UNKNOWN:
> break;
> @@ -3051,7 +3052,6 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
> gdb::array_view<const int> syscall_counts)
> {
> const char *catch_packet;
> - enum packet_status result;
> int n_sysno = 0;
>
> if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
> @@ -3107,8 +3107,8 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
>
> putpkt (catch_packet);
> getpkt (&rs->buf);
> - result = m_features.packet_ok (rs->buf, PACKET_QCatchSyscalls);
> - if (result == PACKET_OK)
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_QCatchSyscalls);
> + if (result.status() == PACKET_OK)
> return 0;
> else
> return -1;
> @@ -5113,7 +5113,8 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
> {
> putpkt ("QStartNoAckMode");
> getpkt (&rs->buf);
> - if (m_features.packet_ok (rs->buf, PACKET_QStartNoAckMode) == PACKET_OK)
> + if ((m_features.packet_ok (rs->buf, PACKET_QStartNoAckMode)).status ()
> + == PACKET_OK)
> rs->noack_mode = 1;
> }
>
> @@ -5898,9 +5899,10 @@ remote_target::remote_query_supported ()
>
> /* If an error occurred, warn, but do not return - just reset the
> buffer to empty and go on to disable features. */
> - if (m_features.packet_ok (rs->buf, PACKET_qSupported) == PACKET_ERROR)
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_qSupported);
> + if (result.status () == PACKET_ERROR)
> {
> - warning (_("Remote failure reply: %s"), rs->buf.data ());
> + warning (_("Remote failure reply: %s"), result.err_msg ());
> rs->buf[0] = 0;
> }
> }
> @@ -6552,7 +6554,8 @@ extended_remote_target::attach (const char *args, int from_tty)
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_vAttach))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_vAttach);
> + switch (result.status ())
> {
> case PACKET_OK:
> if (!target_is_non_stop_p ())
> @@ -6568,9 +6571,9 @@ extended_remote_target::attach (const char *args, int from_tty)
> break;
> case PACKET_UNKNOWN:
> error (_("This target does not support attaching to a process"));
> - default:
> - error (_("Attaching to %s failed"),
> - target_pid_to_str (ptid_t (pid)).c_str ());
> + case PACKET_ERROR:
> + error (_("Attaching to %s failed: %s"),
> + target_pid_to_str (ptid_t (pid)).c_str (), result.err_msg ());
> }
>
> switch_to_inferior_no_thread (remote_add_inferior (false, pid, 1, 0));
> @@ -7493,14 +7496,15 @@ remote_target::remote_interrupt_ns ()
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_vCtrlC))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_vCtrlC);
> + switch (result.status ())
> {
> case PACKET_OK:
> break;
> case PACKET_UNKNOWN:
> error (_("No support for interrupting the remote target."));
> case PACKET_ERROR:
> - error (_("Interrupting target failed: %s"), rs->buf.data ());
> + error (_("Interrupting target failed: %s"), result.err_msg ());
> }
> }
>
> @@ -8796,7 +8800,8 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
>
> buf = rs->buf.data ();
>
> - switch (m_features.packet_ok (rs->buf, PACKET_p))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_p);
> + switch (result.status ())
> {
> case PACKET_OK:
> break;
> @@ -8805,7 +8810,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
> case PACKET_ERROR:
> error (_("Could not fetch register \"%s\"; remote failure reply '%s'"),
> gdbarch_register_name (regcache->arch (), reg->regnum),
> - buf);
> + result.err_msg ());
> }
>
> /* If this register is unfetchable, tell the regcache. */
> @@ -9102,13 +9107,14 @@ remote_target::store_register_using_P (const struct regcache *regcache,
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_P))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_P);
> + switch (result.status ())
> {
> case PACKET_OK:
> return 1;
> case PACKET_ERROR:
> error (_("Could not write register \"%s\"; remote failure reply '%s'"),
> - gdbarch_register_name (gdbarch, reg->regnum), rs->buf.data ());
> + gdbarch_register_name (gdbarch, reg->regnum), result.err_msg ());
> case PACKET_UNKNOWN:
> return 0;
> default:
> @@ -10539,7 +10545,7 @@ remote_target::remote_vkill (int pid)
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_vKill))
> + switch ((m_features.packet_ok (rs->buf, PACKET_vKill)).status ())
> {
> case PACKET_OK:
> return 0;
> @@ -10695,7 +10701,7 @@ remote_target::extended_remote_run (const std::string &args)
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_vRun))
> + switch ((m_features.packet_ok (rs->buf, PACKET_vRun)).status ())
> {
> case PACKET_OK:
> /* We have a wait response. All is well. */
> @@ -10802,11 +10808,14 @@ remote_target::extended_remote_set_inferior_cwd ()
>
> putpkt (rs->buf);
> getpkt (&rs->buf);
> - if (m_features.packet_ok (rs->buf, PACKET_QSetWorkingDir) != PACKET_OK)
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_QSetWorkingDir);
> + if (result.status () == PACKET_ERROR)
> error (_("\
> Remote replied unexpectedly while setting the inferior's working\n\
> directory: %s"),
> - rs->buf.data ());
> + result.err_msg ());
> + if (result.status () == PACKET_UNKNOWN)
> + error (_("Remote target failed to process setting the inferior's working directory"));
>
> }
> }
> @@ -10975,7 +10984,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_Z0))
> + switch ((m_features.packet_ok (rs->buf, PACKET_Z0)).status ())
> {
> case PACKET_ERROR:
> return -1;
> @@ -11076,8 +11085,8 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
> - + to_underlying (packet))))
> + switch ((m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
> + + to_underlying (packet)))).status ())
> {
> case PACKET_ERROR:
> return -1;
> @@ -11125,8 +11134,8 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
> - + to_underlying (packet))))
> + switch ((m_features.packet_ok (rs->buf, (to_underlying (PACKET_Z0)
> + + to_underlying (packet)))).status ())
> {
> case PACKET_ERROR:
> case PACKET_UNKNOWN:
> @@ -11257,7 +11266,6 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
> CORE_ADDR addr = bp_tgt->reqstd_address;
> struct remote_state *rs;
> char *p, *endbuf;
> - char *message;
>
> if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
> return -1;
> @@ -11288,16 +11296,11 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_Z1))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_Z1);
> + switch (result.status ())
> {
> case PACKET_ERROR:
> - if (rs->buf[1] == '.')
> - {
> - message = strchr (&rs->buf[2], '.');
> - if (message)
> - error (_("Remote failure reply: %s"), message + 1);
> - }
> - return -1;
> + error (_("Remote failure reply: %s"), result.err_msg ());
> case PACKET_UNKNOWN:
> return -1;
> case PACKET_OK:
> @@ -11335,7 +11338,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_Z1))
> + switch ((m_features.packet_ok (rs->buf, PACKET_Z1)).status ())
> {
> case PACKET_ERROR:
> case PACKET_UNKNOWN:
> @@ -11376,7 +11379,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
>
> getpkt (&rs->buf);
>
> - status = m_features.packet_ok (rs->buf, PACKET_qCRC);
> + status = (m_features.packet_ok (rs->buf, PACKET_qCRC)).status ();
> if (status == PACKET_ERROR)
> return -1;
> else if (status == PACKET_OK)
> @@ -11498,7 +11501,7 @@ remote_target::remote_write_qxfer (const char *object_name,
>
> if (putpkt_binary (rs->buf.data (), i + buf_len) < 0
> || getpkt (&rs->buf) < 0
> - || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
> + || (m_features.packet_ok (rs->buf, which_packet)).status () != PACKET_OK)
> return TARGET_XFER_E_IO;
>
> unpack_varlen_hex (rs->buf.data (), &n);
> @@ -11563,7 +11566,7 @@ remote_target::remote_read_qxfer (const char *object_name,
> rs->buf[0] = '\0';
> packet_len = getpkt (&rs->buf);
> if (packet_len < 0
> - || m_features.packet_ok (rs->buf, which_packet) != PACKET_OK)
> + || (m_features.packet_ok (rs->buf, which_packet)).status () != PACKET_OK)
> return TARGET_XFER_E_IO;
>
> if (rs->buf[0] != 'l' && rs->buf[0] != 'm')
> @@ -11868,7 +11871,8 @@ remote_target::search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
>
> if (putpkt_binary (rs->buf.data (), i + escaped_pattern_len) < 0
> || getpkt (&rs->buf) < 0
> - || m_features.packet_ok (rs->buf, PACKET_qSearch_memory) != PACKET_OK)
> + || (m_features.packet_ok (rs->buf, PACKET_qSearch_memory)).status ()
> + != PACKET_OK)
> {
> /* The request may not have worked because the command is not
> supported. If so, fall back to the simple way. */
> @@ -12261,7 +12265,6 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
> struct remote_state *rs = get_remote_state ();
> char *p = rs->buf.data ();
> char *endp = p + get_remote_packet_size ();
> - enum packet_status result;
>
> strcpy (p, "qGetTLSAddr:");
> p += strlen (p);
> @@ -12274,15 +12277,15 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
>
> putpkt (rs->buf);
> getpkt (&rs->buf);
> - result = m_features.packet_ok (rs->buf, PACKET_qGetTLSAddr);
> - if (result == PACKET_OK)
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_qGetTLSAddr);
> + if (result.status () == PACKET_OK)
> {
> ULONGEST addr;
>
> unpack_varlen_hex (rs->buf.data (), &addr);
> return addr;
> }
> - else if (result == PACKET_UNKNOWN)
> + else if (result.status () == PACKET_UNKNOWN)
> throw_error (TLS_GENERIC_ERROR,
> _("Remote target doesn't support qGetTLSAddr packet"));
> else
> @@ -12307,7 +12310,6 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
> struct remote_state *rs = get_remote_state ();
> char *p = rs->buf.data ();
> char *endp = p + get_remote_packet_size ();
> - enum packet_status result;
>
> strcpy (p, "qGetTIBAddr:");
> p += strlen (p);
> @@ -12316,8 +12318,8 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
>
> putpkt (rs->buf);
> getpkt (&rs->buf);
> - result = m_features.packet_ok (rs->buf, PACKET_qGetTIBAddr);
> - if (result == PACKET_OK)
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_qGetTIBAddr);
> + if (result.status () == PACKET_OK)
> {
> ULONGEST val;
> unpack_varlen_hex (rs->buf.data (), &val);
> @@ -12325,10 +12327,11 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
> *addr = (CORE_ADDR) val;
> return true;
> }
> - else if (result == PACKET_UNKNOWN)
> + else if (result.status () == PACKET_UNKNOWN)
> error (_("Remote target doesn't support qGetTIBAddr packet"));
> else
> - error (_("Remote target failed to process qGetTIBAddr request"));
> + error (_("Remote target failed to process qGetTIBAddr request, %s"),
> + result.err_msg ());
> }
> else
> error (_("qGetTIBAddr not supported or disabled on this target"));
> @@ -12584,7 +12587,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
> return -1;
> }
>
> - switch (m_features.packet_ok (rs->buf, which_packet))
> + switch ((m_features.packet_ok (rs->buf, which_packet)).status ())
> {
> case PACKET_ERROR:
> *remote_errno = FILEIO_EINVAL;
> @@ -13872,7 +13875,6 @@ remote_target::get_trace_status (struct trace_status *ts)
> {
> /* Initialize it just to avoid a GCC false warning. */
> char *p = NULL;
> - enum packet_status result;
> struct remote_state *rs = get_remote_state ();
>
> if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE)
> @@ -13898,11 +13900,16 @@ remote_target::get_trace_status (struct trace_status *ts)
> throw;
> }
>
> - result = m_features.packet_ok (p, PACKET_qTStatus);
> + packet_result result = m_features.packet_ok (p, PACKET_qTStatus);
>
> - /* If the remote target doesn't do tracing, flag it. */
> - if (result == PACKET_UNKNOWN)
> - return -1;
> + switch (result.status ())
> + {
> + case PACKET_ERROR:
> + error (_("Remote failure reply: %s"), result.err_msg ());
> + /* If the remote target doesn't do tracing, flag it. */
> + case PACKET_UNKNOWN:
> + return -1;
> + }
>
> /* We're working with a live target. */
> ts->filename = NULL;
> @@ -14252,7 +14259,6 @@ remote_target::set_trace_buffer_size (LONGEST val)
> struct remote_state *rs = get_remote_state ();
> char *buf = rs->buf.data ();
> char *endbuf = buf + get_remote_packet_size ();
> - enum packet_status result;
>
> gdb_assert (val >= 0 || val == -1);
> buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
> @@ -14267,10 +14273,15 @@ remote_target::set_trace_buffer_size (LONGEST val)
>
> putpkt (rs->buf);
> remote_get_noisy_reply ();
> - result = m_features.packet_ok (rs->buf, PACKET_QTBuffer_size);
> -
> - if (result != PACKET_OK)
> - warning (_("Bogus reply from target: %s"), rs->buf.data ());
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_QTBuffer_size);
> + switch (result.status ())
> + {
> + case PACKET_ERROR:
> + warning (_("Bogus reply from target: %s"), result.err_msg ());
> + break;
> + case PACKET_UNKNOWN:
> + warning (_("Remote target failed to process the request "));
> + }
> }
> }
>
> @@ -14696,14 +14707,9 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
> putpkt (buf);
> getpkt (&rs->buf);
>
> - if (m_features.packet_ok (buf, PACKET_Qbtrace_conf_bts_size)
> - == PACKET_ERROR)
> - {
> - if (buf[0] == 'E' && buf[1] == '.')
> - error (_("Failed to configure the BTS buffer size: %s"), buf + 2);
> - else
> - error (_("Failed to configure the BTS buffer size."));
> - }
> + packet_result result = m_features.packet_ok (buf, PACKET_Qbtrace_conf_bts_size);
> + if (result.status () == PACKET_ERROR)
> + error (_("Failed to configure the BTS buffer size: %s"), result.err_msg ());
>
> rs->btrace_config.bts.size = conf->bts.size;
> }
> @@ -14719,14 +14725,9 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
> putpkt (buf);
> getpkt (&rs->buf);
>
> - if (m_features.packet_ok (buf, PACKET_Qbtrace_conf_pt_size)
> - == PACKET_ERROR)
> - {
> - if (buf[0] == 'E' && buf[1] == '.')
> - error (_("Failed to configure the trace buffer size: %s"), buf + 2);
> - else
> - error (_("Failed to configure the trace buffer size."));
> - }
> + packet_result result = m_features.packet_ok (buf, PACKET_Qbtrace_conf_pt_size);
> + if (result.status () == PACKET_ERROR)
> + error (_("Failed to configure the trace buffer size: %s"), result.err_msg ());
>
> rs->btrace_config.pt.size = conf->pt.size;
> }
> @@ -14841,15 +14842,10 @@ remote_target::enable_btrace (thread_info *tp,
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - if (m_features.packet_ok (rs->buf, which_packet) == PACKET_ERROR)
> - {
> - if (rs->buf[0] == 'E' && rs->buf[1] == '.')
> - error (_("Could not enable branch tracing for %s: %s"),
> - target_pid_to_str (ptid).c_str (), &rs->buf[2]);
> - else
> - error (_("Could not enable branch tracing for %s."),
> - target_pid_to_str (ptid).c_str ());
> - }
> + packet_result result = m_features.packet_ok (rs->buf, which_packet);
> + if (result.status () == PACKET_ERROR)
> + error (_("Could not enable branch tracing for %s: %s"),
> + target_pid_to_str (ptid).c_str (), result.err_msg ());
>
> btrace_target_info *tinfo = new btrace_target_info { ptid };
>
> @@ -14887,15 +14883,10 @@ remote_target::disable_btrace (struct btrace_target_info *tinfo)
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - if (m_features.packet_ok (rs->buf, PACKET_Qbtrace_off) == PACKET_ERROR)
> - {
> - if (rs->buf[0] == 'E' && rs->buf[1] == '.')
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_Qbtrace_off);
> + if (result.status () == PACKET_ERROR)
> error (_("Could not disable branch tracing for %s: %s"),
> - target_pid_to_str (tinfo->ptid).c_str (), &rs->buf[2]);
> - else
> - error (_("Could not disable branch tracing for %s."),
> - target_pid_to_str (tinfo->ptid).c_str ());
> - }
> + target_pid_to_str (tinfo->ptid).c_str (), result.err_msg ());
>
> delete tinfo;
> }
> @@ -15160,7 +15151,8 @@ remote_target::thread_events (int enable)
> putpkt (rs->buf);
> getpkt (&rs->buf);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_QThreadEvents))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_QThreadEvents);
> + switch (result.status ())
> {
> case PACKET_OK:
> if (strcmp (rs->buf.data (), "OK") != 0)
> @@ -15168,7 +15160,7 @@ remote_target::thread_events (int enable)
> rs->last_thread_events = enable;
> break;
> case PACKET_ERROR:
> - warning (_("Remote failure reply: %s"), rs->buf.data ());
> + warning (_("Remote failure reply: %s"), result.err_msg ());
> break;
> case PACKET_UNKNOWN:
> break;
> @@ -15215,14 +15207,15 @@ remote_target::commit_requested_thread_options ()
> putpkt (rs->buf);
> getpkt (&rs->buf, 0);
>
> - switch (m_features.packet_ok (rs->buf, PACKET_QThreadOptions))
> + packet_result result = m_features.packet_ok (rs->buf, PACKET_QThreadOptions);
> + switch (result.status ())
> {
> case PACKET_OK:
> if (strcmp (rs->buf.data (), "OK") != 0)
> error (_("Remote refused setting thread options: %s"), rs->buf.data ());
> break;
> case PACKET_ERROR:
> - error (_("Remote failure reply: %s"), rs->buf.data ());
> + error (_("Remote failure reply: %s"), result.err_msg ());
> case PACKET_UNKNOWN:
> gdb_assert_not_reached ("PACKET_UNKNOWN");
> break;
> --
> 2.44.0
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-26 9:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-19 13:58 [PATCH v2 1/2] remote.c: Use packet_check_result Alexandra Hájková
2024-03-19 13:58 ` [PATCH v2 2/2] remote.c: Make packet_ok return struct packet_result Alexandra Hájková
2024-03-26 9:48 ` Andrew Burgess
2024-03-25 10:21 ` [PATCH v2 1/2] remote.c: Use packet_check_result Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox