From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A492B3858D34 for ; Thu, 4 Apr 2024 08:52:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A492B3858D34 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A492B3858D34 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712220769; cv=none; b=V04x0kK9p0Aq+20oKLdDYpJ6G3ycuGFBBzae9/Rhb1z+50mKcpeAQZqubS3i548tRdsBy5INjqn7Nyv2ko7XFZiE5m63LK6613nlVYWgxXrlgv7csFNDd0r3KH9+2PJHBFcv6qSrXPe7F3lJ3LgcsxY4GUIVzlj2WAwyNp5Zd0U= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712220769; c=relaxed/simple; bh=Gz2Zj+4J6wmibGFzmYVRK5+T8fD3f0zcHEGj5W1TKzc=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=mBkXetKTQck1CgwC3IqGJs3aMBzt6KjqLRtyzkG7sxyYgbMbVDW6PmyewrXTMFIebUTEjuuLkNlmZ7oGvNW3cI529xrGtp9Tk4m37IrYnPzfNcaR2HJUqggFWrOXHfLHWf3bae0vyoaDJjo8TWGZQizjvQPKkrwSIowKjEhdhho= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712220767; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=07uNERu5dH2vZe9KH1SOcfq7bxcLCKHWNMflCV+ur8g=; b=EzpmM2aAbrKhgVAYVWGjGE3ukP2MDmD3FoHYILt2APESHJ8vHqOdG6LfcnVzsLD9844hLX 9Xu7NSYoMUAC0lMQY3BbYs09vokyDEiFl9+KbwucCFnw8V3Wc7oCD2LP/cSa/rRurGFvRE GJ5m601NRN97zTlcJoQDbyu2wzmkAhw= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-193-28DdhzygOqK2qzJnhrfOrA-1; Thu, 04 Apr 2024 04:52:46 -0400 X-MC-Unique: 28DdhzygOqK2qzJnhrfOrA-1 Received: by mail-ej1-f71.google.com with SMTP id a640c23a62f3a-a4e05cee8faso28502966b.1 for ; Thu, 04 Apr 2024 01:52:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712220763; x=1712825563; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+u7HpypgtmaQdLETLHJWzEtaaHs6wxeGn5tGOTiWTEI=; b=GD5zCvEZtSh3fwFIEe4k3TCzRLwKZ9abkSFJHmFCY+Q+1yi5UiiZedDiYlyRKfSNgu 8gFW/CBUBspDwga6GfnnAJUH1U3jL83m6Nro8UtzpAvNBS94pPuMBa7rU6uSoHaAMOhO XQl4VuA41xll+Z0NslLQ4XApr3eQAwdrcEq98OcvKS5uMwWkx0XPriMRGNy5lK8w6CBC 15x6qnQyxeDR2q2CGjO45wog35kQ5LnkN1HtlVvCe1KZJLxFdvhjpeLPsPDV8Jyi+W0F /1wE+X7FAFGCh7Uvp7hFOSKV0qHtIdjPWZZ8X02Bdp6w0xcySPO7e9GWwvsWMsHZ4uct 3gQA== X-Forwarded-Encrypted: i=1; AJvYcCXQa6jAPbiIJL1SO9/ycMKJEHyXN39dSgUht5SLi/jDE8eYV13wBtJpiKzBsI4Cbt4uKU8t8lYRkQlt6OduZ4HTztnMNpP4q8/j2A== X-Gm-Message-State: AOJu0Yz8A/g3kvc9AAVClM49uxdQzw+6VVnU/xfxHyuNlV9irEW6Hgtr GIL5Jf6ymEx/PJbA4kV13l5+zVVKLBfGEHpwvThozKM4OeSgPmAht49CXfyVMLWGUo336Y5URY+ BW75OJALPQoLX3kdUw0YQUf3Q84lO7h2ZoWu/2XKssAffjgV1MY5thoLseDVJyI9QFg0= X-Received: by 2002:a17:907:6e8b:b0:a4e:adba:4e8c with SMTP id sh11-20020a1709076e8b00b00a4eadba4e8cmr1335837ejc.26.1712220763545; Thu, 04 Apr 2024 01:52:43 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFilY90LOZe+VeBAPjDPUr+CEKO/DxfDGAGPUwjrTPx12ovCkO1gTVR+NDfwmnA4d9/wud7XQ== X-Received: by 2002:a17:907:6e8b:b0:a4e:adba:4e8c with SMTP id sh11-20020a1709076e8b00b00a4eadba4e8cmr1335811ejc.26.1712220762980; Thu, 04 Apr 2024 01:52:42 -0700 (PDT) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id f25-20020a170906561900b00a461b1e814asm8725460ejq.130.2024.04.04.01.52.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Apr 2024 01:52:42 -0700 (PDT) From: Andrew Burgess To: Alexandra =?utf-8?B?SMOhamtvdsOh?= , gdb-patches@sourceware.org Cc: ahajkova@redhat.com Subject: Re: [PATCH v3 1/2] remote.c: Use packet_check_result In-Reply-To: <20240329130059.363456-1-ahajkova@khirnov.net> References: <20240329130059.363456-1-ahajkova@khirnov.net> Date: Thu, 04 Apr 2024 09:52:42 +0100 Message-ID: <87r0fl33xh.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Alexandra H=C3=A1jkov=C3=A1 writes: > From: Alexandra H=C3=A1jkov=C3=A1 > > 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. > > Also use packet_check result in remote_read_bytes_1. Thanks for doing this. Approved-By: Andrew Burgess Thanks, Andrew > --- > gdb/remote.c | 81 +++++++++++++++++++++++++++++----------------------- > 1 file changed, 46 insertions(+), 35 deletions(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index e278711df7b..63f1112095d 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -2450,11 +2450,15 @@ add_packet_config_cmd (const unsigned int which_p= acket, const char *name, > } > } > =20 > -/* 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] !=3D '\0') > { > @@ -2466,14 +2470,20 @@ packet_check_result (const char *buf) > =09/* "Enn" - definitely an error. */ > =09return { PACKET_ERROR, buf + 1 }; > =20 > - /* Always treat "E." as an error. This will be used for > -=09 more verbose error messages, such as E.memtypes. */ > - if (buf[0] =3D=3D 'E' && buf[1] =3D=3D '.') > + /* Not every request accepts an error in a E.msg form. > +=09 Some packets accepts only Enn, in this case E. is not > +=09 defined and E. is treated as PACKET_OK. */ > + if (accept_msg) > =09{ > -=09 if (buf[2] !=3D '\0') > -=09 return { PACKET_ERROR, buf + 2 }; > -=09 else > -=09 return { PACKET_ERROR, "no error provided" }; > +=09 /* Always treat "E." as an error. This will be used for > +=09 more verbose error messages, such as E.memtypes. */ > +=09 if (buf[0] =3D=3D 'E' && buf[1] =3D=3D '.') > +=09 { > +=09 if (buf[2] !=3D '\0') > +=09=09return { PACKET_ERROR, buf + 2 }; > +=09 else > +=09=09return { PACKET_ERROR, "no error provided" }; > +=09 } > =09} > =20 > /* The packet may or may not be OK. Just assume it is. */ > @@ -2487,9 +2497,9 @@ packet_check_result (const char *buf) > } > =20 > 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); > } > =20 > packet_status > @@ -2502,7 +2512,7 @@ remote_features::packet_ok (const char *buf, const = int which_packet) > && config->support =3D=3D PACKET_DISABLE) > internal_error (_("packet_ok: attempt to use a disabled packet")); > =20 > - packet_result result =3D packet_check_result (buf); > + packet_result result =3D packet_check_result (buf, true); > switch (result.status ()) > { > case PACKET_OK: > @@ -8830,7 +8840,7 @@ remote_target::send_g_packet () > xsnprintf (rs->buf.data (), get_remote_packet_size (), "g"); > putpkt (rs->buf); > getpkt (&rs->buf); > - packet_result result =3D packet_check_result (rs->buf); > + packet_result result =3D packet_check_result (rs->buf, true); > if (result.status () =3D=3D PACKET_ERROR) > error (_("Could not read registers; remote failure reply '%s'"), > =09 result.err_msg ()); > @@ -9139,7 +9149,7 @@ remote_target::store_registers_using_G (const struc= t regcache *regcache) > bin2hex (regs, p, rsa->sizeof_g_packet); > putpkt (rs->buf); > getpkt (&rs->buf); > - packet_result pkt_status =3D packet_check_result (rs->buf); > + packet_result pkt_status =3D packet_check_result (rs->buf, true); > if (pkt_status.status () =3D=3D PACKET_ERROR) > error (_("Could not write registers; remote failure reply '%s'"), > =09 pkt_status.err_msg ()); > @@ -9591,9 +9601,8 @@ remote_target::remote_read_bytes_1 (CORE_ADDR memad= dr, gdb_byte *myaddr, > *p =3D '\0'; > putpkt (rs->buf); > getpkt (&rs->buf); > - if (rs->buf[0] =3D=3D 'E' > - && isxdigit (rs->buf[1]) && isxdigit (rs->buf[2]) > - && rs->buf[3] =3D=3D '\0') > + packet_result result =3D packet_check_result (rs->buf, false); > + if (result.status () =3D=3D PACKET_ERROR) > return TARGET_XFER_E_IO; > /* Reply describes memory byte by byte, each byte encoded as two hex > characters. */ > @@ -9747,7 +9756,7 @@ remote_target::remote_send_printf (const char *form= at, ...) > rs->buf[0] =3D '\0'; > getpkt (&rs->buf); > =20 > - return packet_check_result (rs->buf).status (); > + return packet_check_result (rs->buf, true).status (); > } > =20 > /* Flash writing can take quite some time. We'll set > @@ -11930,20 +11939,19 @@ remote_target::rcmd (const char *command, struc= t ui_file *outbuf) > =09 continue; > =09} > buf =3D rs->buf.data (); > - if (buf[0] =3D=3D '\0') > -=09error (_("Target does not support this command.")); > if (buf[0] =3D=3D 'O' && buf[1] !=3D 'K') > =09{ > =09 remote_console_output (buf + 1); /* 'O' message from stub. */ > =09 continue; > =09} > + packet_result result =3D packet_check_result (buf, false); > if (strcmp (buf, "OK") =3D=3D 0) > =09break; > - if (strlen (buf) =3D=3D 3 && buf[0] =3D=3D 'E' > -=09 && isxdigit (buf[1]) && isxdigit (buf[2])) > -=09{ > -=09 error (_("Protocol error with Rcmd")); > -=09} > + else if (result.status () =3D=3D PACKET_UNKNOWN) > +=09error (_("Target does not support this command.")); > + else > +=09error (_("Protocol error with Rcmd: %s."), result.err_msg ()); > + > for (p =3D buf; p[0] !=3D '\0' && p[1] !=3D '\0'; p +=3D 2) > =09{ > =09 char c =3D (fromhex (p[0]) << 4) + fromhex (p[1]); > @@ -15570,7 +15578,7 @@ remote_target::store_memtags (CORE_ADDR address, = size_t len, > getpkt (&rs->buf); > =20 > /* Verify if the request was successful. */ > - return packet_check_result (rs->buf).status () =3D=3D PACKET_OK; > + return packet_check_result (rs->buf, true).status () =3D=3D PACKET_OK; > } > =20 > /* Return true if remote target T is non-stop. */ > @@ -15671,26 +15679,29 @@ static void > test_packet_check_result () > { > std::string buf =3D "E.msg"; > - packet_result result =3D packet_check_result (buf.data ()); > + packet_result result =3D packet_check_result (buf.data (), true); > =20 > SELF_CHECK (result.status () =3D=3D PACKET_ERROR); > SELF_CHECK (strcmp(result.err_msg (), "msg") =3D=3D 0); > =20 > - result =3D packet_check_result ("E01"); > + result =3D packet_check_result ("E01", true); > SELF_CHECK (result.status () =3D=3D PACKET_ERROR); > SELF_CHECK (strcmp(result.err_msg (), "01") =3D=3D 0); > =20 > - SELF_CHECK (packet_check_result ("E1").status () =3D=3D PACKET_OK); > + SELF_CHECK (packet_check_result ("E1", true).status () =3D=3D PACKET_O= K); > =20 > - SELF_CHECK (packet_check_result ("E000").status () =3D=3D PACKET_OK); > + SELF_CHECK (packet_check_result ("E000", true).status () =3D=3D PACKET= _OK); > =20 > - result =3D packet_check_result ("E."); > + result =3D packet_check_result ("E.", true); > SELF_CHECK (result.status () =3D=3D PACKET_ERROR); > SELF_CHECK (strcmp(result.err_msg (), "no error provided") =3D=3D 0); > =20 > - SELF_CHECK (packet_check_result ("some response").status () =3D=3D PAC= KET_OK); > + SELF_CHECK (packet_check_result ("some response", true).status () =3D= =3D PACKET_OK); > + > + SELF_CHECK (packet_check_result ("", true).status () =3D=3D PACKET_UNK= NOWN); > =20 > - SELF_CHECK (packet_check_result ("").status () =3D=3D PACKET_UNKNOWN); > + result =3D packet_check_result ("E.msg", false); > + SELF_CHECK (result.status () =3D=3D PACKET_OK); > } > } // namespace selftests > #endif /* GDB_SELF_TEST */ > --=20 > 2.44.0