bugGNU roff - Bugs: bug #61315, [PATCH] several source files...

 
 

bug #61315: [PATCH] several source files don't include <config.h> first

Submitter:  Paul Eggert <eggert>
Submitted:  Sat 09 Oct 2021 07:51:06 AM UTC
   
 
Category:  General Severity:  3 - Normal
Item Group:  Build/Installation Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 10 Oct 2021 09:00:46 PM UTC, comment #5: 


commit 0c7866632d0490ac452de609c3c4d624a170c8a6
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Sat Oct 9 00:38:54 2021 -0700

    Include <config.h> before any standard headers.

    Gnulib requires that files that might use Gnulib features (which pretty
    much means every C or C++ source file) must include <config.h> first.
    Arrange for that.  This will be needed once Groff updates to the latest
    Gnulib; see Bjarni Ingi Gislason's problem report in:
    https://lists.gnu.org/r/bug-gnulib/2021-10/msg00008.html

    The only exception I can see is src/utils/addftinfo/guess.cpp, which
    does not include any standard include file either directly or
    indirectly, and so need not include <config.h>.

    * src/devices/xditview/Dvi.c:
    * src/devices/xditview/font.c:
    * src/devices/xditview/lex.c:
    * src/devices/xditview/page.c:
    * src/devices/xditview/parse.c:
    * src/libs/libbib/map.c:
    * src/libs/libgroff/change_lf.cpp:
    * src/libs/libgroff/cmap.cpp:
    * src/libs/libgroff/cset.cpp:
    * src/libs/libgroff/fmod.c:
    * src/libs/libgroff/getcwd.c:
    * src/libs/libgroff/lf.cpp:
    * src/libs/libgroff/ptable.cpp:
    * src/libs/libgroff/quotearg.c:
    * src/libs/libxutil/DviChar.c:
    * src/libs/libxutil/XFontName.c:
    * src/libs/libxutil/xmalloc.c:
    * src/utils/indxbib/signal.c: Do it.

    [Fixes <https://savannah.gnu.org/bugs/?61315>. --GBR]


G. Branden Robinson <gbranden>
Group administrator
Sun 10 Oct 2021 07:54:13 PM UTC, comment #4: 

Thanks for following up, Paul.  I'll work on this.  On the bug-groff mailing list, Bjarni reminded us that this is probably the same issue as bug #60339.


G. Branden Robinson <gbranden>
Group administrator
Sat 09 Oct 2021 08:20:13 PM UTC, comment #3: 

The fix for ticket #59276 does not suffice for current Gnulib.

The proposed patch causes Groff to include <config.h> before including any file supplied by the system. This suffices to make Groff robust for both current and future Gnulib, and it is nearly equivalent to Ingo's suggestion to "include config.h everywhere no matter whether it is needed or not". (I write "nearly equivalent" because the proposed patch does not affect src/utils/addftinfo/guess.cpp even though Ingo's suggestion would alter that file.)

Gnulib keeps evolving mostly because POSIX, C, C++, glibc, etc. keep evolving. The Groff source can of course handle this sort of evolution on its own. My suggestion, though, is to take advantage of Gnulib when this saves you work overall, even if there are minor glitches such as the one embodied in the proposed patch.

Paul Eggert <eggert>
Sat 09 Oct 2021 04:02:49 PM UTC, comment #2: 

In ticket #59276 and commit fe121eea, i only fixed those files that actually did require fixing at the time of the commit.  The groff codebase still contains lots of files that do not include config.h.

My rationale for doing the bare minimum was that i deem the intrusiveness and fragility of gnulib utterly disgusting and did not want to pander more to it than really needed.  If some other groff developer wants to deal with this mess and, for example, include config.h everywhere no matter whether it is needed or not, such that in the future, additional worsening of standard conformance in gnulib won't trip us up again, then i won't stand in the way.

I still think it would be better to just get rid of gnulib.  It was probably useful in the 1990ies, but nowadays operating systems worth supporting are usually sufficiently POSIX-compliant that providing a small number of fallback functions by hand is easy without pulling in such a monstrosity.  But i realize i'm probably in the minority round here with that opinion; some have also argued that groff is a GNU project and GNU projects are supposed to use GNU tools no matter what, even if ditching them would make life easier.  <shrug>

Ingo Schwarze <schwarze>
Group Member
Sat 09 Oct 2021 09:42:05 AM UTC, comment #1: 

Hi Paul,

Can you please verify that this was already reported and resolved in bug #59276?

G. Branden Robinson <gbranden>
Group administrator
Sat 09 Oct 2021 07:51:06 AM UTC, original submission:  

Gnulib-using apps are supposed to include <config.h> first, if they use any feature that Gnulib might affect. Since Gnulib redefines standard headers, this means pretty much every source file must include <config.h> first. The Groff source code forgets to do this in some places. Although this is OK with the current groff master branch (commit 2ab0dacb95863a2e347d06cf970676c74c784ce2), it won't work once you update Groff to current Gnulib, as reported by Bjarni Ingi Gislason here:

https://lists.gnu.org/r/bug-gnulib/2021-10/msg00008.html

The obvious fix is to include <config.h> first in every source file that isn't doing that already. Proposed patch attached.

Paul Eggert <eggert>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #52080:  0001-gnulib-Include-config.h-first.patch added by eggert (9KiB - text/x-patch - Modify Groff to include <config.h> first.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by schwarze (Posted a comment)
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by eggert (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-03-05 gbranden CategoryNone General
    2021-10-10 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0
    2021-10-10 gbranden StatusNeed Info In Progress
        Assigned toNone gbranden
        SummarySeveral groff source files don't include &lt;config.h&gt; first [PATCH] several source files don't include <config.h> first
    2021-10-09 gbranden StatusNone Need Info
    2021-10-09 eggert Attached File- Added 0001-gnulib-Include-config.h-first.patch, #52080

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code