bugGNU roff - Bugs: bug #55044, hdtbl: missing space after call to...

 
 

bug #55044: hdtbl: missing space after call to macro t*SP

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Sat 17 Nov 2018 05:04:05 PM UTC
   
 
Category:  Macro - others/general Severity:  2 - Minor
Item Group:  Incorrect behaviour Status:  Confirmed
Privacy:  Public Assigned to:  None
Open/Closed:  Open Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Sat 24 Nov 2018 04:46:39 PM UTC, comment #3: 

Obviously, changing anything below examples/ cannot fix an issue in the macro files themselves, so the obviously useless patch in comment #2 was not even worth testing.  Posting patches that you already know to be wrong is not a good idea.

> I was ONLY assessing the use of the undefined variable "M#",
> nothing else as the original subject tells.


That is a serious problem right there.  Don't do that, ever.

Auditing compiler warnings is a TOOL for finding bugs.  If you are not prepared to actually inspect the code the compiler warns about in order to understand what it is trying to do and in order to judge whether it is doing that correctly and/or how it should be improved in order to reach correct results, don't bother looking at compiler warnings.  The crucial part of a fix for a compiler warning is to make sure that the code is actually doing what it intends to do.

The main goal of auditing compiler warnings is NOT to silence the warnings.  Auditing them with that objective in mind is worse than useless.  The outcome of "fixing" compiler warnings in that mindset usually is hiding existing bugs AND introducing new bugs.

That said, we still don't know how to fix the actual bug that the compiler warning revealed.

Ingo Schwarze <schwarze>
Group Member
Sun 18 Nov 2018 07:08:05 PM UTC, comment #2: 

  I first defined "M#" in the macro "HM" (head... macro) as I assumed
that this macro would be processed first (before the other choice,
"t*SP"):


commit 4b4998eb759c215bcf9a1c474f608b3add840337
Author: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date:   Fri Nov 16 00:58:40 2018 +0000

    contrib/examples/common.roff: define register M#

      The register "M#" is undefined when first used.

      Define it in the macro HM (header macro).

    Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>

diff --git a/contrib/hdtbl/examples/common.roff b/contrib/hdtbl/examples/common.roff
index b40a9d16..e26a78f5 100644
--- a/contrib/hdtbl/examples/common.roff
+++ b/contrib/hdtbl/examples/common.roff
@@ -257,9 +257,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .
 .
 .de HM
-.  sp |.5i                                \"        print header in top margin
+.  sp |.5i                        \"        print header in top margin
 .  tl \\*[t*HM]
 .  sp |\\n[tH]u
+.  nr M# 0                        \"        used in hdmisc.tmac, macro t*SP (SP)
 .  ev
 ..
 .

###

  As there was no change in the use of the undefined variable I tried
the patch for "t*SP, which worked.

  I was ONLY assessing the use of the undefined variable "M#",
nothing else as the original subject tells.


Bjarni Ingi Gislason <bjarniig>
Sat 17 Nov 2018 11:21:01 PM UTC, comment #1: 

This is not the first time that i'm telling you that you keep spamming us with trivial patches without understanding what you are doing.  Even though these patches are trivial, this one is again incorrect.

Simple code inspection shows that there is an actual bug here, and your patch would only make that bug harder to find.

Consider the following test file:

some
.br
lines
.br
of
.br
text
.t*SP
.bp
one line only
.br
.t*SP
missing space before this line

Expected output should have a blank line between "one line only" and "missing space before this line".  Actual output lacks that blank line.  The reason for this bug is that contrary to the (german) comment, \n[M#] is not reset at the start of a new page.  The macro .HM mentioned in that comment doesn't even exist, it was deleted eight years ago.  But even before that, it did not reset \n[M#] - the bug is likely present since import in 2006 and was likely introduced by pre-import changes of the original authors.

It is not completely clear to me yet how to fix it.  Somehow, \n[M#] must be reset to zero when a new page starts.  The same bug might affect columnated text as well, so it may also need resetting at the start of a new column.

The low code quality of the hdtbl macros is really annoying, they are buggy as hell, and the missing initializations are the least among the problems.

Ingo Schwarze <schwarze>
Group Member
Sat 17 Nov 2018 05:04:05 PM UTC, original submission:  

From 8c1d8c684874ef967f856cb6edca4ed1ea41cfc5 Mon Sep 17 00:00:00 2001
From: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date: Fri, 16 Nov 2018 01:39:04 +0000
Subject: [PATCH] contrib/hdtbl/hdmisc.tmac-u: Initialize register 'M#'

  The register 'M#' has been used undefined.  Fix that.

Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
 contrib/hdtbl/hdmisc.tmac-u | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


diff --git a/contrib/hdtbl/hdmisc.tmac-u b/contrib/hdtbl/hdmisc.tmac-u
index 5dd952bf..00e9de2e 100644
--- a/contrib/hdtbl/hdmisc.tmac-u
+++ b/contrib/hdtbl/hdmisc.tmac-u
@@ -191,7 +191,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .\" Initialize register '<<'.  The meaning is unknown.  It is only
 .\"   used once (undefined) in the macro "t*SP".
 .nr << 0
-
+.
+.\" Initialize register 'M#'
+.nr M# 0
+.
 .de t*SP
 .  if (\\n[nl] < 0) \
 .    br                        \"        start very first page
--
2.19.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 bgarrigues (Updated the item)
  • -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
    2018-11-25 bgarrigues Severity3 - Normal 2 - Minor
        Summary[PATCH] contrib/hdtbl/hdmisc.tmac-u: register 'M#' must be reset to 0 at the start of a new page hdtbl: missing space after call to macro t*SP
    2018-11-17 schwarze Item GroupFeature change Incorrect behaviour
        StatusNone Confirmed
        Summary[PATCH] contrib/hdtbl/hdmisc.tmac-u: Initialize register 'M#' [PATCH] contrib/hdtbl/hdmisc.tmac-u: register 'M#' must be reset to 0 at the start of a new page

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code