From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id TG1QHetArmX46QMAWB0awg (envelope-from ) for ; Mon, 22 Jan 2024 05:18:19 -0500 Authentication-Results: simark.ca; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=amd.com header.i=@amd.com header.a=rsa-sha256 header.s=selector1 header.b=Um5nPdWg; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 6390B1E0C3; Mon, 22 Jan 2024 05:18:19 -0500 (EST) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 27B1C1E098 for ; Mon, 22 Jan 2024 05:18:17 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6C76A3857833 for ; Mon, 22 Jan 2024 10:18:16 +0000 (GMT) Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10on2068.outbound.protection.outlook.com [40.107.93.68]) by sourceware.org (Postfix) with ESMTPS id B9C883858414 for ; Mon, 22 Jan 2024 10:17:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B9C883858414 Authentication-Results: sourceware.org; dmarc=fail (p=quarantine dis=none) header.from=amd.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=amd.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B9C883858414 Authentication-Results: server2.sourceware.org; arc=fail smtp.remote-ip=40.107.93.68 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1705918666; cv=fail; b=ETgkL1Ht2LFXvfE3nodobSpGWTbm73G/lvBaBOKZv4R2wU+q97j4NwTnkDG40VbZyZUJDRGIEFXQJR6vFGsR2b7MA2KRvS5HXoEs9n7Hv/WCTuIzekcwAZAwxnfqdc4vWNkp/L8kvgeJN+iUvVU2Z/RVrbxbC8T+gxT6XSQwxR0= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1705918666; c=relaxed/simple; bh=IQ9dLP1+J5WmAv70naRpV3TfEbKt1tidveDQ8oJtG+w=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=LtoU/UG6AdYjF7hCuDtGkUWvcURHCA7V5jk5dffV9C6HD1lWjAF2QXxglBJsaFOAHhxNyKL5JE+VeXrQkqhUDt2GNpeaEpYlIRV1syK0T6/mre0opHTv3DP9fVagAydexQSttrekQz91sWM28ozroTFFLxRDCRBses/NfR3AmVY= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AYW2LZS+AlrLqgYS3hzIvHWZJRyRJNQ7UDFWi9gI7LACunhp0+4MegnETM+iby8+7D6AWa7hyB+SmaxUDmHdNw6P8qb4SyQ/azAeOfGcqgugJG0iUGEcwcPwuOgjUscE72FyG8SEOCqHDy2uCFIM4N+3JRHUrkwa5mQAbojtrAKKN0WKltHw3hKGVaZTTcDUsCXpkYQjLesPpj5/1EOOEH7NwiCUXnmtkTEXi1q6BU5RC4cQWRtvPrLyf3PvZOIRWxhvU5RS0qG2QSO2ufrDVqHK+YyBCsUhe50t53y9PVLVuo80Uqzcd3uN3wzXMHiCUkeOnDua2Cgld3r36dkTEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=d0UgY4KozwSJ3uyriqvJO4H3aiq6vbnFO1EhPlO/fcI=; b=NNkTUh/rdazS0cC0AuZLW+qVKKwcX2tPxdbDjFUqTjTO+Uo7m7qnEfI7shOOJQvZyKBK22IPFK+DSlsjEAkxSQYUvsmicuqKyqlxaEpi4k9yQRcdfRXFM08TxE1lospffh9Cx16JP2k8ZQMtoYGiKwQ32xfxZ89unKfCkshNwDnCU4nk/ieqIc/ixhyLR3A1z/qKjxUT2ZfoVlmoRa3GTi0w2VwWkyLfrjniqwop+KKSkt59vAPRLZ3lW7t4L2iY/5lXErrV2Z02jwsAA/OJ35aS2FsiybC+7Upr0kOXOjv4QRoydSPMGVbVLbPcaS/iloywhBjyJZer0bQnwVIu0A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=redhat.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=d0UgY4KozwSJ3uyriqvJO4H3aiq6vbnFO1EhPlO/fcI=; b=Um5nPdWgOXX46bb2Cqq14a/FYYC4VDGiOuBQCPdBM6i7BvVu8RQQ9rGk+qohTgfe7hG8SiVwxd8m6ZqrCA3dZYgaTtYRtEtCoXMGvo+k8yW7aVX0GGjs24oy/C7TkrVEGnTVwsMEjtzzOrwfoDVTgICH3SxN1DyGIH40bR101qs= Received: from PH0PR07CA0091.namprd07.prod.outlook.com (2603:10b6:510:4::6) by SN7PR12MB6839.namprd12.prod.outlook.com (2603:10b6:806:265::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7202.32; Mon, 22 Jan 2024 10:17:40 +0000 Received: from SN1PEPF0002BA52.namprd03.prod.outlook.com (2603:10b6:510:4:cafe::df) by PH0PR07CA0091.outlook.office365.com (2603:10b6:510:4::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7202.32 via Frontend Transport; Mon, 22 Jan 2024 10:17:40 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by SN1PEPF0002BA52.mail.protection.outlook.com (10.167.242.75) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.7202.16 via Frontend Transport; Mon, 22 Jan 2024 10:17:40 +0000 Received: from khazad-dum (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Mon, 22 Jan 2024 04:17:39 -0600 Date: Mon, 22 Jan 2024 10:17:30 +0000 From: Lancelot SIX To: Alexandra =?utf-8?B?SMOhamtvdsOh?= CC: Subject: Re: [PATCH v3] remote.c: Make packet_check_result return a structure Message-ID: <20240122101730.xejdz5wbb2thpgw5@khazad-dum> References: <20240121145051.70453-1-ahajkova@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240121145051.70453-1-ahajkova@redhat.com> X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB04.amd.com (10.181.40.145) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SN1PEPF0002BA52:EE_|SN7PR12MB6839:EE_ X-MS-Office365-Filtering-Correlation-Id: 1074fd47-1b6f-4cf1-4f0b-08dc1b335d8f X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 9wygRu0z7hh+51F0eL/VDRxneP06ZGaaXx2xKfN25/CqTPcOQ5lR/lZlO7q42kdQfrUNczbpqsQgG6aBfCso4yECLXSonPkzP8MEhQTU7uZOZnECh42j24nUf4cIS5IcpmmJ50k4MHmQOfMVphmc9cTplnvy6xC+xxNfH4MjZbVl6/53AKEHOVjM6jWqD4c2w3YdNTJMS8KQXxb5CdhQ5FWZHcfQTkSor9lregpToAmqO7t/GWK/Qf3WARhLC4P9sTCKRLoN5TcbMSjcEpAaISVD9cs21Tgyj1dGsbNp1UVMfQim1KXx0LNM3cE3O42KjJkVRHAvbdYsh2lMRaCG+oQbHW8ModMmxa+nmEcOOitqV4+E7UFiD72D65WyH+tiSL7+k19J3u4aZ0503FOyHKMDRwns0H2r9MbenTUFDStOarf4v2I45SqgwGTkFRj8WS1mMO8EZgIzsBqFu6acqlGKXNLJliSW8JcT0yyf8ATu4rUfqihBYqdgA0rnqAn8MU9HGQ/MBv2k+gCFfoYZBU5mXPTwOxJvhhT69pOfE6MYmbud+1huGqOfWEbPf2BmJHkuHQI3zu5WJSsrJHOUAQppxOWZ+OPSy+ljpWFMvd/ZfPckCExhXCXvMAaiNxACN5TgTL8uQuaAIrcWJmQGcEaT7BW1Us15Uc0lFzK7hLJf+z9scbv7lpyjxzfQHTHSSx/qdNbkDsSnich2VafAvneLss2jxC4ByA2R/wFBGSzhaHx0t/PSN1M/wOq2t2ullLZbl3c0CgXow6ZvwNB4NUAavdETLsgdblF9U3tlQ1NRysY7qNT/9wHnvbxIuylJ X-Forefront-Antispam-Report: CIP:165.204.84.17; CTRY:US; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:SATLEXMB04.amd.com; PTR:InfoDomainNonexistent; CAT:NONE; SFS:(13230031)(4636009)(39860400002)(346002)(376002)(396003)(136003)(230173577357003)(230273577357003)(230922051799003)(82310400011)(451199024)(64100799003)(1800799012)(186009)(46966006)(40470700004)(36840700001)(1076003)(26005)(16526019)(33716001)(83380400001)(66574015)(426003)(336012)(478600001)(6666004)(9686003)(41300700001)(36860700001)(40460700003)(2906002)(8936002)(86362001)(70206006)(70586007)(316002)(8676002)(4326008)(6916009)(5660300002)(356005)(55016003)(40480700001)(81166007)(30864003)(82740400003)(47076005)(36900700001); DIR:OUT; SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jan 2024 10:17:40.6097 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 1074fd47-1b6f-4cf1-4f0b-08dc1b335d8f X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d; Ip=[165.204.84.17]; Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: SN1PEPF0002BA52.namprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB6839 X-Spam-Status: No, score=-15.2 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, FORGED_SPF_HELO, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Hi Alexandra, I have no experience with remote, but I have a couple of styling and other minor remarks below. On Sun, Jan 21, 2024 at 03:50:24PM +0100, Alexandra Hájková wrote: > packet_check_result currently returns an packet_result enum. > If GDB will recieve an error in a format E.errtext, which > is possible for some q packets, such errtext is lost if > treated by packet_check_result. > Introduce a new structure which bundles enum packet_result > with possible error message or error code returned by > the GDBserver. > I plan to make more GDBserver response checking functions to use > packet_check_result to make remote.c code more consistent. > This will also allow to use E.errtext more in the future. > > There's no infrastructure to test this with a test case so > I tested this by modifying store_registers_using_G function > to get an error message: > > [remote] Sending packet: $GG00000000 ... > > gdbserver: Wrong sized register packet (expected 1792 bytes, got 1793) > gdbserver: Invalid hex digit 71 > Killing process(es): 1104002 > [remote] Packet received: E01 > Could not write registers; remote failure reply '01' > > Reviewed-by: Thiago Jung Bauermann > --- > v3: - Improve indentation > > gdb/remote.c | 148 ++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 112 insertions(+), 36 deletions(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index 72f14e28f54..8fe459a4b14 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -18,8 +18,7 @@ > along with this program. If not, see . */ > > /* See the GDB User Guide for details of the GDB remote protocol. */ > - > -#include "defs.h" > + #include "defs.h" This looks a spurious and undesired change. > #include > #include > #include "inferior.h" > @@ -149,13 +148,46 @@ get_target_type_name (bool target_connected) > /* Analyze a packet's return value and update the packet config > accordingly. */ > > -enum packet_result > +enum packet_status > { > PACKET_ERROR, > PACKET_OK, > PACKET_UNKNOWN > }; > > +/* Keeps packet's return value. If packet's return value is PACKET_ERROR, ^ I think the coding standard asks for two spaces here. > + err_msg contains an error message string from E.string or the number > + stored as a string from E.num. */ > +struct packet_result > +{ > + packet_result (enum packet_status status, std::string err_msg) > + :m_status (status), m_err_msg (std::move (err_msg)) ^ We usually have a space between the ":" and the following token: > + { > + gdb_assert (status == PACKET_ERROR); > + } > + > + explicit packet_result (enum packet_status status) > + :m_status (status) ^ Same here. > + { > + gdb_assert (status != PACKET_ERROR); > + } > + > + enum packet_status status () const > + { > + return this->m_status; > + } > + > + const char *err_msg () const > + { > + gdb_assert (this->m_status == PACKET_ERROR); > + return this->m_err_msg.c_str (); > + } > + > +private: > + enum packet_status m_status; > + std::string m_err_msg; > +}; > + > /* Enumeration of packets for a remote target. */ > > enum { > @@ -732,8 +764,8 @@ struct remote_features > > /* Check result value in BUF for packet WHICH_PACKET and update the packet's > support configuration accordingly. */ > - packet_result packet_ok (const char *buf, const int which_packet); > - packet_result packet_ok (const gdb::char_vector &buf, const int which_packet); > + packet_status packet_ok (const char *buf, const int which_packet); > + packet_status 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; > @@ -1254,7 +1286,7 @@ class remote_target : public process_stratum_target > int unit_size, > ULONGEST *xfered_len); > > - packet_result remote_send_printf (const char *format, ...) > + packet_status remote_send_printf (const char *format, ...) > ATTRIBUTE_PRINTF (2, 3); > > target_xfer_status remote_flash_write (ULONGEST address, > @@ -2418,7 +2450,10 @@ add_packet_config_cmd (const unsigned int which_packet, const char *name, > } > } > > -static enum packet_result > +/* Check GDBserver's reply packet. Return packet_result ^ Missing a space here. > + structure which contains the packet_status enum > + and an error message for the PACKET_ERROR case. */ > +static packet_result > packet_check_result (const char *buf) > { > if (buf[0] != '\0') > @@ -2428,42 +2463,44 @@ packet_check_result (const char *buf) > if (buf[0] == 'E' > && isxdigit (buf[1]) && isxdigit (buf[2]) > && buf[3] == '\0') > + { > /* "Enn" - definitely an error. */ > - return PACKET_ERROR; > + return { PACKET_ERROR, buf + 1 }; > + } The lines inside the { } block are under-intended. > > /* 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] == '.') > - return PACKET_ERROR; > + return { PACKET_ERROR, buf + 2 }; > > /* The packet may or may not be OK. Just assume it is. */ > - return PACKET_OK; > + return packet_result ( PACKET_OK ); > } > else > + { > /* The stub does not support the packet. */ > - return PACKET_UNKNOWN; > + return packet_result ( PACKET_UNKNOWN ); > + } > } > > -static enum packet_result > +static packet_result > packet_check_result (const gdb::char_vector &buf) > { > return packet_check_result (buf.data ()); > } > > -packet_result > +packet_status > remote_features::packet_ok (const char *buf, const int which_packet) > { > packet_config *config = &m_protocol_packets[which_packet]; > packet_description *descr = &packets_descriptions[which_packet]; > > - enum packet_result result; > - > if (config->detect != AUTO_BOOLEAN_TRUE > && config->support == PACKET_DISABLE) > internal_error (_("packet_ok: attempt to use a disabled packet")); > > - result = packet_check_result (buf); > - switch (result) > + packet_result result = packet_check_result (buf); > + switch (result.status ()) > { > case PACKET_OK: > case PACKET_ERROR: > @@ -2498,10 +2535,10 @@ remote_features::packet_ok (const char *buf, const int which_packet) > break; > } > > - return result; > + return result.status (); > } > > -packet_result > +packet_status > remote_features::packet_ok (const gdb::char_vector &buf, const int which_packet) > { > return packet_ok (buf.data (), which_packet); > @@ -3000,7 +3037,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count, > gdb::array_view syscall_counts) > { > const char *catch_packet; > - enum packet_result result; > + enum packet_status result; > int n_sysno = 0; > > if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE) > @@ -8791,9 +8828,10 @@ remote_target::send_g_packet () > xsnprintf (rs->buf.data (), get_remote_packet_size (), "g"); > putpkt (rs->buf); > getpkt (&rs->buf); > - if (packet_check_result (rs->buf) == PACKET_ERROR) > + packet_result result = packet_check_result (rs->buf); > + if (result.status () == PACKET_ERROR) > error (_("Could not read registers; remote failure reply '%s'"), > - rs->buf.data ()); > + result.err_msg ()); > > /* We can get out of synch in various cases. If the first character > in the buffer is not a hex character, assume that has happened > @@ -9099,9 +9137,10 @@ remote_target::store_registers_using_G (const struct regcache *regcache) > bin2hex (regs, p, rsa->sizeof_g_packet); > putpkt (rs->buf); > getpkt (&rs->buf); > - if (packet_check_result (rs->buf) == PACKET_ERROR) > + packet_result pkt_status = packet_check_result (rs->buf); > + if (pkt_status.status () == PACKET_ERROR) > error (_("Could not write registers; remote failure reply '%s'"), > - rs->buf.data ()); > + pkt_status.err_msg ()); > } > > /* Store register REGNUM, or all registers if REGNUM == -1, from the contents > @@ -9683,7 +9722,7 @@ remote_target::remote_read_bytes (CORE_ADDR memaddr, > FORMAT and the remaining arguments, then gets the reply. Returns > whether the packet was a success, a failure, or unknown. */ > > -packet_result > +packet_status > remote_target::remote_send_printf (const char *format, ...) > { > struct remote_state *rs = get_remote_state (); > @@ -9705,8 +9744,9 @@ remote_target::remote_send_printf (const char *format, ...) > > rs->buf[0] = '\0'; > getpkt (&rs->buf); > + packet_result pkt_status = packet_check_result (rs->buf); > > - return packet_check_result (rs->buf); > + return pkt_status.status (); Maybe it could just be return packet_check_result (rs->buf).status (); > } > > /* Flash writing can take quite some time. We'll set > @@ -9718,7 +9758,7 @@ void > remote_target::flash_erase (ULONGEST address, LONGEST length) > { > int addr_size = gdbarch_addr_bit (current_inferior ()->arch ()) / 8; > - enum packet_result ret; > + enum packet_status ret; > scoped_restore restore_timeout > = make_scoped_restore (&remote_timeout, remote_flash_timeout); > > @@ -11308,7 +11348,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size > if (target_has_execution () > && m_features.packet_support (PACKET_qCRC) != PACKET_DISABLE) > { > - enum packet_result result; > + enum packet_status status; > > /* Make sure the remote is pointing at the right process. */ > set_general_process (); > @@ -11324,10 +11364,10 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size > > getpkt (&rs->buf); > > - result = m_features.packet_ok (rs->buf, PACKET_qCRC); > - if (result == PACKET_ERROR) > + status = m_features.packet_ok (rs->buf, PACKET_qCRC); > + if (status == PACKET_ERROR) > return -1; > - else if (result == PACKET_OK) > + else if (status == PACKET_OK) > { > for (target_crc = 0, tmp = &rs->buf[1]; *tmp; tmp++) > target_crc = target_crc * 16 + fromhex (*tmp); > @@ -12210,7 +12250,7 @@ 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_result result; > + enum packet_status result; > > strcpy (p, "qGetTLSAddr:"); > p += strlen (p); > @@ -12256,7 +12296,7 @@ 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_result result; > + enum packet_status result; > > strcpy (p, "qGetTIBAddr:"); > p += strlen (p); > @@ -13821,7 +13861,7 @@ remote_target::get_trace_status (struct trace_status *ts) > { > /* Initialize it just to avoid a GCC false warning. */ > char *p = NULL; > - enum packet_result result; > + enum packet_status result; > struct remote_state *rs = get_remote_state (); > > if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE) > @@ -14201,7 +14241,7 @@ 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_result result; > + enum packet_status result; > > gdb_assert (val >= 0 || val == -1); > buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:"); > @@ -15527,9 +15567,10 @@ remote_target::store_memtags (CORE_ADDR address, size_t len, > > putpkt (rs->buf); > getpkt (&rs->buf); > + packet_result pkt_status = packet_check_result (rs->buf); > > /* Verify if the request was successful. */ > - return packet_check_result (rs->buf.data ()) == PACKET_OK; > + return pkt_status.status () == PACKET_OK; Similarly, maybe it could be return packet_check_result (rs->buf).status () == PACKET_OK; > } > > /* Return true if remote target T is non-stop. */ > @@ -15626,6 +15667,39 @@ test_memory_tagging_functions () > expected.length ()) == 0); > } > > +static void > +test_packet_check_result () > +{ > + std::string buf = "E01"; > + > + packet_result result = packet_check_result (buf.data ()); > + SELF_CHECK (result.status () == PACKET_ERROR); > + > + buf.assign("E1"); > + result = packet_check_result (buf.data ()); > + SELF_CHECK (result.status () == PACKET_OK); > + > + buf.assign("E000"); > + result = packet_check_result (buf.data ()); > + SELF_CHECK (result.status () == PACKET_OK); > + > + buf.assign("E.msg"); > + result = packet_check_result (buf.data ()); > + SELF_CHECK (result.status () == PACKET_ERROR); > + > + buf.assign("E."); > + result = packet_check_result (buf.data ()); > + SELF_CHECK (result.status () == PACKET_ERROR); > + > + buf.assign("some response"); > + result = packet_check_result (buf.data ()); > + SELF_CHECK (result.status () == PACKET_OK); > + > + buf.assign(""); > + result = packet_check_result (buf.data ()); > + SELF_CHECK (result.status () == PACKET_UNKNOWN); For the non-PACKET_ERROR cases, it might be shorter to just write SELF_CHECK (packet_check_result ("E1").status () == PACKET_OK); For the PACKET_ERROR cases, maybe also include tests checking packet_status::err_msg () ? Best, Lancelot. > +} > + > } // namespace selftests > #endif /* GDB_SELF_TEST */ > > @@ -16125,5 +16199,7 @@ from the target."), > #if GDB_SELF_TEST > selftests::register_test ("remote_memory_tagging", > selftests::test_memory_tagging_functions); > + selftests::register_test ("packet_check_result", > + selftests::test_packet_check_result); > #endif > } > > -- > 2.43.0 >