bugGNU roff - Bugs: bug #55449, [PATCH] Use FILENAME_MAX in...

 
 

bug #55449: [PATCH] Use FILENAME_MAX in maxfilename.cpp

Submitter:  Eli Zaretskii <eliz>
Submitted:  Fri 11 Jan 2019 07:41:05 PM UTC
   
 
Category:  Core Severity:  3 - Normal
Item Group:  Incorrect behaviour Status:  Fixed
Privacy:  Public Assigned to:  schwarze
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 05 Feb 2023 06:58:42 AM UTC, comment #7: 


commit 9c5f9a862a7522665d9f84e38d86aef5e588e879
Author: Ingo Schwarze <schwarze@openbsd.org>
Date:   Wed Oct 21 18:31:24 2020 +0200

    On systems without NAME_MAX, use FILENAME_MAX as a last resort.

    Problem reported by Eli Zaretskii <eliz at gnu dot org>
    on MS Windows in https://savannah.gnu.org/bugs/?55449
    OK gbranden@ eliz@


G. Branden Robinson <gbranden>
Group administrator
Wed 21 Oct 2020 10:32:15 PM UTC, comment #6: 

Pushed in 9c5f9a862a7522665d9f84e38d86aef5e588e879.
Thanks for checking!

Ingo Schwarze <schwarze>
Group Member
Wed 21 Oct 2020 05:02:07 PM UTC, comment #5: 

Yes, the patch works, thanks.

Eli Zaretskii <eliz>
Wed 21 Oct 2020 04:43:27 PM UTC, comment #4: 

The patch #45943 is not correct.

According to the comment at the top of src/libs/libgroff/maxfilename.cpp, what the function file_name_max() wants to return is NAME_MAX.

If a system defines both NAME_MAX and FILENAME_MAX - which all system ought to do because POSIX requires both - then the patch clobbers NAME_MAX with FILENAME_MAX.  On some systems, both have different values.

I also suggest to use a more readable idiom avoiding nested #ifs, see the attached patch 55449-schwarze.patch.

Also, use FILENAME_MAX only as a last-resort fallback.  It is not clear to me that FILENAME_MAX has exactly the same meaning as NAME_MAX.  On some systems, FILENAME_MAX is much larger than NAME_MAX, so using it does not seem perfectly safe.  Then again, when nothing else is available, maybe it is still better than 14.

Eli, can you test whether this version works for you, too?

(file #50036)

Ingo Schwarze <schwarze>
Group Member
Wed 21 Oct 2020 02:23:09 PM UTC, comment #3: 

Granted, the patch was tested, and works in the port of Groff I use every day since that report.

But maybe you wanted a "second opinion"?

Eli Zaretskii <eliz>
Wed 21 Oct 2020 07:27:40 AM UTC, comment #2: 

Do we have someone with access to a Windows machine to test this on?

Eli filed it almost immediately after the previous release; it'd be nice to do something about it before the next one.

G. Branden Robinson <gbranden>
Group administrator
Sat 12 Jan 2019 08:06:46 AM UTC, comment #1: 

A simple patch to fix this is in the attachment.


(file #45943)

Eli Zaretskii <eliz>
Fri 11 Jan 2019 07:41:05 PM UTC, original submission:  

For platforms which don't have the Posix pathconf, maxfilename.cpp should try harder, and look at (the ANSI-standard) FILENAME_MAX in addition to the other varieties.  Right now, it decides that NAME_MAX is 14 on MS-Windows, which of course makes no sense.  One adverse effect of this is that temporary file names don't get the "groff" prefix, for no good reason.

Eli Zaretskii <eliz>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50036:  55449-schwarze.patch added by schwarze (2KiB - text/x-patch)
file #45943:  max_name.patch added by eliz (362B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by schwarze (Updated the item)
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by eliz (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
    2020-10-21 schwarze StatusNeed Info Fixed
        Open/ClosedOpen Closed
    2020-10-21 schwarze Attached File- Added 55449-schwarze.patch, #50036
        Assigned toNone schwarze
        Planned ReleaseNone 1.23.0
    2020-10-21 gbranden StatusNone Need Info
    2020-10-13 barx Carbon-CopyRemoved 93119 -
    2020-10-13 barx SummaryUse FILENAME_MAX in maxfilename.cpp [PATCH] Use FILENAME_MAX in maxfilename.cpp
    2019-01-12 eliz Attached File- Added max_name.patch, #45943

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code