From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) by sourceware.org (Postfix) with ESMTPS id 8BD66384AB66 for ; Tue, 23 Apr 2024 18:58:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8BD66384AB66 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8BD66384AB66 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.54 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713898706; cv=none; b=RiBa1JEn9ZGZFeoiIIHmztFXkv7vs79LYqVqEIWww58HconHLuj/WvNexk4h/qmwOrIBD7LF84bJP7hKJ/eX/jd0y321kPm/pD9HjkTsKmuf9CSHH6kdWOpim853vvCY1zQeX5Ur0w+3RLDWFJlEXl2UV0lKstDI4hIOppSKi84= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713898706; c=relaxed/simple; bh=LY2cLBTcQSqUiMChvYlbZouOafCeiBGF/71X4h+KiH8=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=XU4mKhqJ8YcJpxzFW31ta2PFXBNh4Jy1iaM0ILfNnM2pcTxfxdLYoBBJ0eCnAVnKcztDRrm95HmUjK5iB9AjB3Amcb6j23AMjYang5+E7D2q30U6snXpR3DXqDxSyRK6MrsQIBQyALS1An1uaLVwtN7g5qWFGmGEtuM4HRWB4Es= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-41a72f3a1edso19127095e9.2 for ; Tue, 23 Apr 2024 11:58:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713898701; x=1714503501; h=content-transfer-encoding:in-reply-to:content-language:from :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=q6VU1dx+OtHxDhNMOulXlKtacGBc+4VLb/3lSj5/PIM=; b=JaFHiUk5YpTL/f4Q3uBJExw3jtYyo/wkCudATQZxTlcwNJuedl2gUmyAQ6HZX42Scs zaiOBWVp2o7YfAO/uHwTlnJBP2mjp6bBuGKGaESQEGeowDCFXSlXDbF4m3b+Vy9+ayYp rLpjYnIGiELY9RKR3OU2HI/WXc4298g8rJBXTMKFo/eTVPf4lK6ATZs3GC/wNe60d5HL X3A1drQ5cLZ5zht5GXk0prSnAfF4NV4Qd7+Re/vKgnPaN5s8TiNzxzpsf6ea0rlWoZsR VRWdW032HuOP/NDHyZggAMcoV92QZJdBmyoQqx4NMziJ4Zns/oo39d/3JRwDgTkMEUVK ivxQ== X-Forwarded-Encrypted: i=1; AJvYcCUY4Ad01owV3Z9QieOOv9qkenRdxIdkFSuQzm4Ees/8DtySj3CIz+yHsuuu85gZbyqlT/jeUfRuQCDhPlFl1LwMhg7EVQe8vtCtNw== X-Gm-Message-State: AOJu0Yw2fgEr2JymIT+N4CYg3n0+dTqpAcmH/hNAlefsvLX2ppXg3o3r PtqsToVa/Z03Oj1LDnxIjaYAhFx5KxeYWjfITTYPAw8fpM4Ls/hB4NanXw== X-Google-Smtp-Source: AGHT+IGpFCSwrwK73jNdWAyaAo5d8Lz/G6nrofIiOZvdLhEzfKmuRU8EnQvPkeAcZmrWxRUKYT/eZA== X-Received: by 2002:a05:600c:3504:b0:418:dbad:c57d with SMTP id h4-20020a05600c350400b00418dbadc57dmr104973wmq.28.1713898701136; Tue, 23 Apr 2024 11:58:21 -0700 (PDT) Received: from ?IPV6:2001:8a0:f93d:b900:4f86:481:20cb:27d? ([2001:8a0:f93d:b900:4f86:481:20cb:27d]) by smtp.gmail.com with ESMTPSA id h15-20020a056000000f00b003434c764f01sm15234495wrx.107.2024.04.23.11.58.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Apr 2024 11:58:20 -0700 (PDT) Message-ID: <5ff21602-f875-4eda-8507-12a48bcbe340@palves.net> Date: Tue, 23 Apr 2024 19:58:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] gdb/doc: Fix incorrect information in RSP doc To: Ciaran Woodward , gdb-patches@sourceware.org References: <20240422153502.7250-1-ciaranwoodward@xmos.com> From: Pedro Alves Content-Language: en-US In-Reply-To: <20240422153502.7250-1-ciaranwoodward@xmos.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,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: On 2024-04-22 16:35, Ciaran Woodward wrote: > The 'PacketSize' attribute of the qSupported packet was > documented to be the maximum size of the packet including > the frame and checksum bytes, however this is not how it > was treated in the code. In reality, PacketSize is the > maximum size of the data in the RSP packets, not including > the framing or checksum bytes. > > For instance, GDB's remote.c treats it as the maximum > number of data bytes. See remote_read_bytes_1, where the > size of the request is capped at PacketSize/2 (for > hex-encoding). OTOH, we have code like this: /* Should rsa->sizeof_g_packet needs more space than the default, adjust the size accordingly. Remember that each byte is encoded as two characters. 32 is the overhead for the packet header / footer. NOTE: cagney/1999-10-26: I suspect that 8 (``$NN:G...#NN'') is a better guess, the below has been padded a little. */ if (this->sizeof_g_packet > ((this->remote_packet_size - 32) / 2)) this->remote_packet_size = (this->sizeof_g_packet * 2 + 32); and this: remote_target::remote_write_bytes_aux () ... payload_capacity_bytes = get_memory_write_packet_size (); /* The packet buffer will be large enough for the payload; get_memory_packet_size ensures this. */ rs->buf[0] = '\0'; /* Compute the size of the actual payload by subtracting out the packet header and footer overhead: "$M,:...#nn". */ payload_capacity_bytes -= strlen ("$,:#NN"); So looks like we have a mess? Most code in remote.c seems to assume get_remote_packet_size() returns the max payload size, except, not always. I think it would be good to rename remote_packet_size to remote_packet_data_size or remote_packet_payload_size and similarly rename the few functions around this, like get_remote_packet_size and get_memory_write_packet_size, and fix those cases above accordingly. > > Also see gdbserver's server.cc, where the internal buffer > is sized as PBUFSIZ and PBUFSIZ-1 is used as PacketSize. > In gdbserver's case, the buffer is not used for any of the > framing or checksum characters. (I am not certain where the -1 > comes from. I think it comes from back when there were no > binary packets, so packets were treated as strings with > null terminators). > > It also seems like gdbservers in the wild treat it in > this way: > > Embocosm doc: > https://www.embecosm.com/appnotes/ean4/embecosm-howto-rsp-server-ean4-issue-2.html#id3078000 > > A quick glance over openocd's gdb_server.c gdb_put_packet_inner() > function shows that the internal buffer also excludes the framing > and checksum. > > Likewise, qEmu's gdbstub.c allocates PacketSize bytes for > the internal packet contents, and PacketSize+4 for the > full frame. > --- > gdb/doc/gdb.texinfo | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 31a531ee992..b2e9faac82d 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -44953,7 +44953,7 @@ These are the currently defined stub features, in more detail: > The remote stub can accept packets up to at least @var{bytes} in > length. @value{GDBN} will send packets up to this size for bulk > transfers, and will never send larger packets. This is a limit on the > -data characters in the packet, including the frame and checksum. > +data characters in the packet, not including the frame and checksum. Yes, to me, this just looks like a typo. "data characters in the packet" reads to me like talking about what the docs refer to as packet-data (i.e., the payload), and the "not including the frame and checksum." would be just there to try say redundant things to be clear. The sentence just doesn't make sense as is -- the frame and checksum are not part of the data characters, so "including" is a strange word to use. If it really was trying to say the whole set of characters in the packet, it would have been phrased differently IMO, like saying "plus" instead of "including, perhaps. With the "not", it now serves the redundancy purpose I'm referring. Approved-By: Pedro Alves