[7/8] Use @deftypefn in chew output

Message ID 20230208071725.3668898-8-tom@tromey.com
State Accepted
Headers
Series Make the BFD info manual a bit prettier |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Tom Tromey Feb. 8, 2023, 7:17 a.m. UTC
  When reading the BFD info manual, function definitions looked very
strange to me:

    *Synopsis*
	 long bfd_get_mtime (bfd *abfd);
       *Description*
    Return the file modification time (as read from the file system, or from
    the archive header for archive members).

The *Synopsis* and *Description* text in particular is very un-info-like.

To fix this, I tried removing the *Synopsis* text and having FUNCTION
use @deftypefn instead.  However, this ended up requiring some new
state, because SYNOPSIS can appear without FUNCTION.  This in turn
required "catstrif" (I considered adding FORTH-style if-else-then, but
in the end decided on an ad hoc approach).

After this the result looks like:

 -- Function: long bfd_get_mtime (bfd *abfd);
     Return the file modification time (as read from the file system, or
     from the archive header for archive members).

This patch also reorders a few documentation comments to ensure that
SYNOPSIS comes before DESCRIPTION.  This is the more common style and
is also now required by doc.str.

bfd/ChangeLog
2023-02-07  Tom Tromey  <tom@tromey.com>

	* syms.c (bfd_decode_symclass, bfd_is_undefined_symclass)
	(bfd_symbol_info): Reorder documentation comment.
	* doc/doc.str (synopsis_seen): New variable.
	(SYNOPSIS): Set synopsis_seen.  Emit @deftypefn.
	(DESCRIPTION): Use synopsis_seen.
	* doc/chew.c (catstrif): New function.
	(main): Add catstrif intrinsic.
	(compile): Recognize "variable" command.
---
 bfd/ChangeLog   | 11 +++++++++++
 bfd/doc/chew.c  | 30 ++++++++++++++++++++++++++++++
 bfd/doc/doc.str | 14 +++++++++-----
 bfd/syms.c      | 18 +++++++++---------
 4 files changed, 59 insertions(+), 14 deletions(-)
  

Comments

Simon Marchi Feb. 15, 2023, 5:55 p.m. UTC | #1
On 2/8/23 02:17, Tom Tromey wrote:
> When reading the BFD info manual, function definitions looked very
> strange to me:
> 
>     *Synopsis*
> 	 long bfd_get_mtime (bfd *abfd);
>        *Description*
>     Return the file modification time (as read from the file system, or from
>     the archive header for archive members).
> 
> The *Synopsis* and *Description* text in particular is very un-info-like.
> 
> To fix this, I tried removing the *Synopsis* text and having FUNCTION
> use @deftypefn instead.  However, this ended up requiring some new
> state, because SYNOPSIS can appear without FUNCTION.  This in turn
> required "catstrif" (I considered adding FORTH-style if-else-then, but
> in the end decided on an ad hoc approach).
> 
> After this the result looks like:
> 
>  -- Function: long bfd_get_mtime (bfd *abfd);
>      Return the file modification time (as read from the file system, or
>      from the archive header for archive members).
> 
> This patch also reorders a few documentation comments to ensure that
> SYNOPSIS comes before DESCRIPTION.  This is the more common style and
> is also now required by doc.str.
> 
> bfd/ChangeLog
> 2023-02-07  Tom Tromey  <tom@tromey.com>
> 
> 	* syms.c (bfd_decode_symclass, bfd_is_undefined_symclass)
> 	(bfd_symbol_info): Reorder documentation comment.
> 	* doc/doc.str (synopsis_seen): New variable.
> 	(SYNOPSIS): Set synopsis_seen.  Emit @deftypefn.
> 	(DESCRIPTION): Use synopsis_seen.
> 	* doc/chew.c (catstrif): New function.
> 	(main): Add catstrif intrinsic.
> 	(compile): Recognize "variable" command.

Hi Tom,

Starting with this commit, I see this when building (with ASan enabled):

  GEN      doc/aoutx.stamp

=================================================================
==45648==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 14 byte(s) in 1 object(s) allocated from:
    #0 0x7f193eebfa89 in __interceptor_malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x55a292052ba0 in xmalloc /home/smarchi/src/binutils-gdb/bfd/doc/chew.c:158
    #2 0x55a29205678a in nextword /home/smarchi/src/binutils-gdb/bfd/doc/chew.c:1090
    #3 0x55a292057c1e in compile /home/smarchi/src/binutils-gdb/bfd/doc/chew.c:1331
    #4 0x55a292058b8b in main /home/smarchi/src/binutils-gdb/bfd/doc/chew.c:1511
    #5 0x7f193ec3c78f  (/usr/lib/libc.so.6+0x2378f)

Simon
  
Tom Tromey Feb. 15, 2023, 11:11 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Starting with this commit, I see this when building (with ASan enabled):

Oops, sorry about that.  This patch should fix it.  Let me know what you
think.

Tom

commit 482e9a3e3a3aee643d671d2594be709446cef065
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Feb 15 16:09:35 2023 -0700

    Avoid memory leak in chew
    
    An earlier patch of mine introduced a memory leak in chew.  The bug
    was that the new "variable" word didn't free the following word.  This
    patch fixes it by arranging to transfer ownership of the name to the
    variable itself.
    
    bfd/ChangeLog
    2023-02-15  Tom Tromey  <tom@tromey.com>
    
            * doc/chew.c (add_variable): New function, from
            add_intrinsic_variable.
            (add_intrinsic_variable): Call add_variable.
            (compile): Call add_variable.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index e160e4472df..5bf3e9f0919 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@
+2023-02-15  Tom Tromey  <tom@tromey.com>
+
+	* doc/chew.c (add_variable): New function, from
+	add_intrinsic_variable.
+	(add_intrinsic_variable): Call add_variable.
+	(compile): Call add_variable.
+
 2023-02-07  Tom Tromey  <tom@tromey.com>
 
 	* bfd.c, cache.c, compress.c, opncls.c: Remove RETURNS from
diff --git a/bfd/doc/chew.c b/bfd/doc/chew.c
index 19e3781bdda..cd399697abd 100644
--- a/bfd/doc/chew.c
+++ b/bfd/doc/chew.c
@@ -1241,9 +1241,9 @@ add_intrinsic (char *name, void (*func) (void))
 }
 
 static void
-add_intrinsic_variable (char *name, intptr_t *loc)
+add_variable (char *name, intptr_t *loc)
 {
-  dict_type *new_d = newentry (xstrdup (name));
+  dict_type *new_d = newentry (name);
   pcu p = { push_variable };
   add_to_definition (new_d, p);
   p.l = (intptr_t) loc;
@@ -1252,6 +1252,12 @@ add_intrinsic_variable (char *name, intptr_t *loc)
   add_to_definition (new_d, p);
 }
 
+static void
+add_intrinsic_variable (const char *name, intptr_t *loc)
+{
+  add_variable (xstrdup (name), loc);
+}
+
 void
 compile (char *string)
 {
@@ -1333,7 +1339,7 @@ compile (char *string)
 	    continue;
 	  intptr_t *loc = xmalloc (sizeof (intptr_t));
 	  *loc = 0;
-	  add_intrinsic_variable (word, loc);
+	  add_variable (word, loc);
 	  string = nextword (string, &word);
 	}
       else
  
Simon Marchi Feb. 16, 2023, 1:29 a.m. UTC | #3
On 2/15/23 18:11, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> Starting with this commit, I see this when building (with ASan enabled):
> 
> Oops, sorry about that.  This patch should fix it.  Let me know what you
> think.
> 
> Tom

It works for me, thanks!

Simon
  
Alan Modra Feb. 19, 2023, 3:46 a.m. UTC | #4
On Wed, Feb 15, 2023 at 08:29:55PM -0500, Simon Marchi via Binutils wrote:
> 
> 
> On 2/15/23 18:11, Tom Tromey wrote:
> >>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> > 
> > Simon> Starting with this commit, I see this when building (with ASan enabled):
> > 
> > Oops, sorry about that.  This patch should fix it.  Let me know what you
> > think.
> > 
> > Tom
> 
> It works for me, thanks!
> 
> Simon

I've pushed the patch now.
  

Patch

diff --git a/bfd/doc/chew.c b/bfd/doc/chew.c
index b17b20a183f..19e3781bdda 100644
--- a/bfd/doc/chew.c
+++ b/bfd/doc/chew.c
@@ -31,6 +31,9 @@ 
    You define new words thus:
    : <newword> <oldwords> ;
 
+   Variables are defined using:
+   variable NAME
+
 */
 
 /* Primitives provided by the program:
@@ -67,6 +70,7 @@ 
 	outputdots - strip out lines without leading dots
 	maybecatstr - do catstr if internal_mode == internal_wanted, discard
 		value in any case
+	catstrif - do catstr if top of integer stack is nonzero
 	translatecomments - turn {* and *} into comment delimiters
 	kill_bogus_lines - get rid of extra newlines
 	indent
@@ -1015,6 +1019,20 @@  maybecatstr (void)
   pc++;
 }
 
+static void
+catstrif (void)
+{
+  int cond = isp[0];
+  isp--;
+  icheck_range ();
+  if (cond)
+    catstr (tos - 1, tos);
+  delete_string (tos);
+  tos--;
+  check_range ();
+  pc++;
+}
+
 char *
 nextword (char *string, char **word)
 {
@@ -1307,6 +1325,17 @@  compile (char *string)
 	  free (word);
 	  string = nextword (string, &word);
 	}
+      else if (strcmp (word, "variable") == 0)
+	{
+	  free (word);
+	  string = nextword (string, &word);
+	  if (!string)
+	    continue;
+	  intptr_t *loc = xmalloc (sizeof (intptr_t));
+	  *loc = 0;
+	  add_intrinsic_variable (word, loc);
+	  string = nextword (string, &word);
+	}
       else
 	{
 	  fprintf (stderr, "syntax error at %s\n", string - 1);
@@ -1444,6 +1473,7 @@  main (int ac, char *av[])
   add_intrinsic ("swap", swap);
   add_intrinsic ("outputdots", outputdots);
   add_intrinsic ("maybecatstr", maybecatstr);
+  add_intrinsic ("catstrif", catstrif);
   add_intrinsic ("translatecomments", translatecomments);
   add_intrinsic ("kill_bogus_lines", kill_bogus_lines);
   add_intrinsic ("indent", indent);
diff --git a/bfd/doc/doc.str b/bfd/doc/doc.str
index 6019b7ccfdb..4576d497521 100644
--- a/bfd/doc/doc.str
+++ b/bfd/doc/doc.str
@@ -16,6 +16,9 @@ 
 -  along with this program; if not, write to the Free Software
 -  Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
 
+-  True if SYNOPSIS was seen.
+variable synopsis_seen
+
 : DOCDD
 	skip_past_newline
 	get_stuff_in_command kill_bogus_lines catstr
@@ -48,14 +51,12 @@ 
 
 : SYNOPSIS
 	skip_past_newline
-	"@strong{Synopsis}\n" catstr
-	"@example\n" catstr
+	1 synopsis_seen !
+	"@deftypefn {Function} " catstr
 	get_stuff_in_command  
 	kill_bogus_lines
 	indent
 	catstr
-	"@end example\n" catstr
-
 	;
 
 : func
@@ -118,7 +119,10 @@ 
 
 	
 : DESCRIPTION 
-	"@strong{Description}@*\n" catstr subhead ;
+	subhead
+	"@end deftypefn\n" synopsis_seen @ catstrif
+	0 synopsis_seen !
+	;
 
 : RETURNS
 	"@strong{Returns}@*\n" catstr subhead ;
diff --git a/bfd/syms.c b/bfd/syms.c
index a26aeb4cb8b..c460e377723 100644
--- a/bfd/syms.c
+++ b/bfd/syms.c
@@ -642,12 +642,12 @@  decode_section_type (const struct bfd_section *section)
 FUNCTION
 	bfd_decode_symclass
 
+SYNOPSIS
+	int bfd_decode_symclass (asymbol *symbol);
+
 DESCRIPTION
 	Return a character corresponding to the symbol
 	class of @var{symbol}, or '?' for an unknown class.
-
-SYNOPSIS
-	int bfd_decode_symclass (asymbol *symbol);
 */
 int
 bfd_decode_symclass (asymbol *symbol)
@@ -725,13 +725,13 @@  bfd_decode_symclass (asymbol *symbol)
 FUNCTION
 	bfd_is_undefined_symclass
 
+SYNOPSIS
+	bool bfd_is_undefined_symclass (int symclass);
+
 DESCRIPTION
 	Returns non-zero if the class symbol returned by
 	bfd_decode_symclass represents an undefined symbol.
 	Returns zero otherwise.
-
-SYNOPSIS
-	bool bfd_is_undefined_symclass (int symclass);
 */
 
 bool
@@ -744,13 +744,13 @@  bfd_is_undefined_symclass (int symclass)
 FUNCTION
 	bfd_symbol_info
 
+SYNOPSIS
+	void bfd_symbol_info (asymbol *symbol, symbol_info *ret);
+
 DESCRIPTION
 	Fill in the basic info about symbol that nm needs.
 	Additional info may be added by the back-ends after
 	calling this function.
-
-SYNOPSIS
-	void bfd_symbol_info (asymbol *symbol, symbol_info *ret);
 */
 
 void