From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88755 invoked by alias); 10 Sep 2018 18:43:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 88633 invoked by uid 89); 10 Sep 2018 18:43:42 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=closely, referred, baldwin, Baldwin X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 10 Sep 2018 18:43:40 +0000 Received: from John-Baldwins-MacBook-Pro-2.local (ralph.baldwin.cx [66.234.199.215]) by mail.baldwin.cx (Postfix) with ESMTPSA id D9EA110B670; Mon, 10 Sep 2018 14:43:38 -0400 (EDT) Subject: Re: [PATCH 5/5] Document the 'info proc files' command. To: Eli Zaretskii References: <20180908003659.37482-1-jhb@FreeBSD.org> <20180908003659.37482-6-jhb@FreeBSD.org> <83a7ostqvd.fsf@gnu.org> Cc: gdb-patches@sourceware.org From: John Baldwin Message-ID: <1d069bee-7706-fe4c-3190-3b1a6c40a14d@FreeBSD.org> Date: Mon, 10 Sep 2018 18:43:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <83a7ostqvd.fsf@gnu.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00294.txt.bz2 On 9/8/18 12:01 AM, Eli Zaretskii wrote: >> From: John Baldwin >> Date: Fri, 7 Sep 2018 17:36:59 -0700 >> >> diff --git a/gdb/NEWS b/gdb/NEWS >> index 75436b0fc3..f5ea98ac52 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -51,6 +51,9 @@ maint set dwarf unwinders (on|off) >> maint show dwarf unwinders >> Control whether DWARF unwinders can be used. >> >> +info proc files >> + Display a list of open files for a process. >> + > > This is OK. > >> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo >> index f2d1155b4d..42c077aa69 100644 >> --- a/gdb/doc/gdb.texinfo >> +++ b/gdb/doc/gdb.texinfo >> @@ -22236,6 +22236,14 @@ supported on @sc{gnu}/Linux and FreeBSD. >> Show the name of executable of the process. This command is supported >> on @sc{gnu}/Linux and FreeBSD. >> >> +@item info proc files >> +@cindex info proc files >> +Report the open file descriptors accessible in the program. Each > > Not "accessible in", "open by", right? "Accessible" is ambiguous, it > could mean "can potentially be accessed", and that is not what is > shown here, AFAIU. Ok. I was probably trying to match the text from 'info proc mappings' too closely. Also, in the same vein, this should really say "process" instead of "program" I think. (The mappings one says program currently but should probably also say "process" instead.) (Oh, you already included the "process" fix below as well.) >> +entry displays the index and type of each descriptor. The file name > > By "index", I guess you meant the value of the descriptor, is that > right? Yes, not sure if there's a better way to describe that. Looks like you used "number" below and that is probably fine. >> +is also listed for descriptors with an associated file name. Network >> +socket descriptors display the socket addresses in place of the file >> +name. > > From the example you have shown (btw, why not show it in the manual?), > I understand that the Name field is always present, and is either a > file or directory name, or the protocol and socket address. If that > is indeed correct, I suggest to reword the description: > > Show the file descriptors open by the process. For each open file > descriptor, @value{GDBN} shows its number, type (file, directory, > character device, socket), offset, and the name of the resource open > on the descriptor. The resource name can be a file name (for files, > directories, and devices) or a protocol followed by socket address > (for network connections). This looks better to me, thanks. > This lacks the details about "offset", which you didn't describe, and > I couldn't guess. Some file descriptors (for files for example) include a read/write offset (the thing lseek() changes) used as the starting offset of read() and write() (but not for syscalls like pread() and pwrite() that take an explicit offset). This value is usually referred to as the "offset" of the file descriptor (e.g. in man pages for lseek). >> This command is supported on FreeBSD. > > Only on FreeBSD? My patch series only adds support for FreeBSD. I expect someone else will probably implement this on at least Linux. I used this language to match existing language of other 'info proc' subcommands in the manual (e.g. 'info proc cmdline/cwd/exe/status' all include a sentence saying "This command is supported on @sc{gnu}/Linux and FreeBSD.") I don't mind adjusting it, I just think we should be consistent in the language we use. If we want to use something more future-proof we might add a blanket statement to the "info proc" node to say that individual subcommands are only supported on some systems or something to that effect. -- John Baldwin                                                                            Â