bugGNU roff - Bugs: bug #57592, add "include guards" to...

 
 

bug #57592: add "include guards" to macro files

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Mon 13 Jan 2020 06:36:55 PM UTC
   
 
Category:  Macro - others/general Severity:  1 - Wish
Item Group:  Feature change Status:  Rejected
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 30 Mar 2023 05:18:50 PM UTC, comment #8: 

Dropping patch annotation from rejected ticket.

G. Branden Robinson <gbranden>
Group administrator
Tue 04 Feb 2020 07:57:47 AM UTC, comment #7: 

comment #5:

> And while I'm here I will add that doing the above would be irritating for `an-old.tmac` at least, which pledges itself to classical troff compatibility and therefore has matchbox-sized namespace.


Amusingly, my correction comment introduced an even bigger error than the one I was fixing in the first place.

`an-old.tmac` is not written in classical troff; it deploys many GNU roff features and uses prodigiously long register names that for all I know cause buffer overflows in Heirloom troff.  Somewhere my brain knew this, because I mentioned a couple of them in the groff_man(7) page.

It is `an-ext.tmac` that is written in the compatible dialect.

G. Branden Robinson <gbranden>
Group administrator
Thu 16 Jan 2020 06:24:45 AM UTC, comment #6: 

I don't think you need to be sorry for a minor imperfection in the rationale for closing an invalid or irrelevant ticket.

Bug triaging and keeping a bugtracker clean causes a substantial amount of work.  That is especially true when there is a source of substantial numbers of low-quality tickets.

While it is important that commits are well-tested and documentation and commit messages are accurate, i think dealing with unimportant tickets should be optimized for speed, not for precision, nor does the rationale need to be detailed.

Thanks, Branden, for doing the work.

Ingo Schwarze <schwarze>
Group Member
Thu 16 Jan 2020 06:09:14 AM UTC, comment #5: 


> I regret my lack of clarity in characterizing my objection earlier.  In fact I think our "include guards" could be improved by keying of a simple that is only there for the purpose of detecting multiple inclusion.


s/keying of a simple/keying off a symbol/

And while I'm here I will add that doing the above would be irritating for `an-old.tmac` at least, which pledges itself to classical troff compatibility and therefore has matchbox-sized namespace.

G. Branden Robinson <gbranden>
Group administrator
Thu 16 Jan 2020 06:06:41 AM UTC, comment #4: 

To be fair to Bjarni, he wasn't requesting a change in the semantics of `.nx`, but asking us to add include guards to some of the macro packages.

But as Ingo pointed out, the macro files should be written idempotently.  Having them skip to the next file of input when a symbol (string, macro, diversion, request) that the package defines is already defined is a pretty conventional way to do this and I do not think requires a warning.

I regret my lack of clarity in characterizing my objection earlier.  In fact I think our "include guards" could be improved by keying of a simple that is only there for the purpose of detecting multiple inclusion.

But better still would be to simply have "reentrant" macro packages. :P

G. Branden Robinson <gbranden>
Group administrator
Wed 15 Jan 2020 03:43:11 AM UTC, comment #3: 

I concur with Ingo and Steffen.

The semantics of `.nx` are really simple--it's a "goto".  It doesn't even return to the original file as `.so` does.

In any case, if people need include guards for roff files, they'll have to implement them.

If someone wants to request a GNU extension request to allow file-reading with `#pragma once` semantics à la C from GCC 3.4 and later, that can be filed as a separate wishlist bug, preferably with some consideration to design impact.

But I'm a NAK on changing the semantics of `.nx`, which goes back to [CSTR #54](http://cm.bell-labs.com/cm/cs/cstr/54.ps.gz).

G. Branden Robinson <gbranden>
Group administrator
Wed 15 Jan 2020 03:35:45 AM UTC, comment #2: 

For the benefit of Savannah readers, here is part of Steffen's comment:

> I do not think this is a good idea in general, since this means you cannot blindly include some file to ensure it is present or not.  Say you want to include some picture in a report and include the macro set which allows that, but it was already sourced (maybe  due to some condition) in another macro set.  Now you have a pointless warning.


https://lists.gnu.org/archive/html/bug-groff/2020-01/msg00019.html

G. Branden Robinson <gbranden>
Group administrator
Tue 14 Jan 2020 12:18:03 PM UTC, comment #1: 

I strongly agree with Steffen Nurpmeso on this one.  Let me quote what he wrote on the mailing list:

"I do not think this is a good idea in general, since this means you cannot blindly include some file to ensure it is present or not.  Say you want to include some picture in a report and include the macro set which allows that, but it was already sourced (maybe due to some condition) in another macro set.  Now you have a pointless warning.  The way it is done right now is how most languages ensure that files get included once only once [...]"

Besides, there is no danger in including a file twice as long as the file acts in an idempotent manner.  So Steffen explained above why the patch is detrimental, and this additional observation means that it has very little benefit, if any.

Finally, this is complicating code without a good reason, which also isn't a good idea.

Can someone provide an OK to close this ticket as "invalid"?

Thanks, Ingo

Ingo Schwarze <schwarze>
Group Member
Mon 13 Jan 2020 06:36:55 PM UTC, original submission:  


From b42eb50920646823ea06c423da73510c708a23c1 Mon Sep 17 00:00:00 2001
From: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date: Mon, 13 Jan 2020 18:10:39 +0000
To: bug-groff@gnu.org
Subject: [PATCH] tmac/<files containing a ".nx" request>: Add a warning

  Some macro files have an '.nx' request
to avoid to have all of it read a second time.

  Add a warning to point out this unwanted situation.

  Two files, "pdfpic.tmac" and "pspic.tmac",
are automatically read,
as they are used in the file "tmac/troffrc".

Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
 tmac/an-old.tmac | 5 ++++-
 tmac/devtag.tmac | 5 ++++-
 tmac/doc.tmac-u  | 5 ++++-
 tmac/e.tmac-u    | 5 ++++-
 tmac/pdfpic.tmac | 6 +++++-
 tmac/pspic.tmac  | 6 +++++-
 tmac/s.tmac      | 5 ++++-
 tmac/www.tmac.in | 5 ++++-
 8 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/tmac/an-old.tmac b/tmac/an-old.tmac
index ba360013..171d2814 100644
--- a/tmac/an-old.tmac
+++ b/tmac/an-old.tmac
@@ -37,7 +37,10 @@
 .
 .if !\n(.g .ab These man macros work only with groff.
 .
-.do if d RI .nx
+.do if d RI \{\
+.  tm \n[.F]: warning: The macro file is read again.
+.  nx
+.\}
 .
 .do mso devtag.tmac
 .
diff --git a/tmac/devtag.tmac b/tmac/devtag.tmac
index 80a5dde2..f691f733 100644
--- a/tmac/devtag.tmac
+++ b/tmac/devtag.tmac
@@ -43,7 +43,10 @@ called directly from within www.tmac.  The tags defined here are
 reasonably generic and could be applied to other devices.
 ..
 .
-.do if d DEVTAG-NH .nx
+.do if d DEVTAG-NH \{\
+.  tm \n[.F]: warning: The macro file is read again
+.  nx
+.\}
 .
 .nr _C \n(.C
 .cp 0
diff --git a/tmac/doc.tmac-u b/tmac/doc.tmac-u
index a8b66283..df5680bc 100644
--- a/tmac/doc.tmac-u
+++ b/tmac/doc.tmac-u
@@ -51,7 +51,10 @@
 .  ab This version of mdoc can be run with GNU troff only!
 .
 .
-.do if d Dd .nx
+.do if d Dd \{\
+.  tm \n[.F]: warning: The macro file is read again
+.  nx
+.\}
 .
 .
 .cp 0
diff --git a/tmac/e.tmac-u b/tmac/e.tmac-u
index 572fba8b..d5e8b160 100644
--- a/tmac/e.tmac-u
+++ b/tmac/e.tmac-u
@@ -78,7 +78,10 @@
 .        do als check_need_title @check_need_title
 .
 .@R pf
-.if \n(pf .nx
+.if \n(pf \{\
+.  tm \n[.F]: warning: The macro file is read again
+.  nx
+.\}
 .
 .if !\n(.g .ig
 .de @S                \" --- initialize string/macro to empty, if undefined
diff --git a/tmac/pdfpic.tmac b/tmac/pdfpic.tmac
index fd2b9f3c..726a5920 100644
--- a/tmac/pdfpic.tmac
+++ b/tmac/pdfpic.tmac
@@ -12,7 +12,11 @@
 .\" Requires the poppler-utils package (for pdfinfo and pdftops).
 .\" Requires running groff in unsafe mode.
 .
-.do if d PDFPIC .nx
+.do if d PDFPIC \{\
+.\" This file is first read in "tmac/troffrc"
+.  tm \n[.F]: warning: The macro file is read again
+.  nx
+.\}
 .
 .nr _C \n(.C
 .cp 0
diff --git a/tmac/pspic.tmac b/tmac/pspic.tmac
index 948754f4..5082c465 100644
--- a/tmac/pspic.tmac
+++ b/tmac/pspic.tmac
@@ -9,7 +9,11 @@
 .\"
 .\"   .PSPIC [-L|-R|-C|-I <indent>] <file> [<width> [<height>]]
 .
-.do if d PSPIC .nx
+.do if d PSPIC \{\
+.\" This file is first read in "tmac/troffrc"
+.  tm \n[.F]: warning: The macro file is read again.
+.  nx
+.\}
 .
 .nr _C \n(.C
 .cp 0
diff --git a/tmac/s.tmac b/tmac/s.tmac
index 08f18cf2..e18ff0a5 100644
--- a/tmac/s.tmac
+++ b/tmac/s.tmac
@@ -37,7 +37,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .\}
 .
 .\" See if already loaded.
-.if r GS .nx
+.if r GS \{\
+.  tm \n[.F]: warning: The macro file is read again
+.  nx
+.\}
 .mso devtag.tmac
 .nr GS 1
 .nr need_eo_h 0
diff --git a/tmac/www.tmac.in b/tmac/www.tmac.in
index c3a2928e..2d807d8d 100644
--- a/tmac/www.tmac.in
+++ b/tmac/www.tmac.in
@@ -44,7 +44,10 @@ following, HTML always denotes XHTML also.
 .\" Setup
 .\" --------------------------------------------------------------------
 .
-.do if d www:lenstr .nx
+.do if d www:lenstr \{\
+.  tm \n[.F]: warning: The macro file is read again
+.  nx
+.\}
 .
 .do mso devtag.tmac
 .
--
2.24.1


Bjarni Ingi Gislason <bjarniig>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by schwarze (Posted a comment)
  • -email is unavailable- added by bjarniig (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-03-30 gbranden Summary[PATCH] tmac/&lt;files containing a &quot;.nx&quot; request&gt;: Add a warning add "include guards" to macro files
    2020-01-15 gbranden StatusNone Rejected
        Assigned toNone gbranden
        Open/ClosedOpen Closed
    2020-01-14 schwarze Severity3 - Normal 1 - Wish

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code