bugGNU nano - Bugs: bug #50289, slight mistreatment of...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #50289: slight mistreatment of <ctype.h> functions

Submitter:  None
Submitted:  Sun 12 Feb 2017 06:11:27 PM UTC
   
 
Severity:  2 - Minor Status:  Fixed
Assigned to:  bens Open/Closed:  Closed

Sun 02 Apr 2017 09:48:22 AM UTC, comment #5: 

The changes were released in 2.8.0.

Benno Schulenberg <bens>
Group administrator
Mon 06 Mar 2017 08:18:48 PM UTC, comment #4: 

Applied to git, 636b7348.  Thanks.

Benno Schulenberg <bens>
Group administrator
Fri 24 Feb 2017 07:08:35 PM UTC, comment #3: 

The change itself is mainly an issue of formal correctness, but it can indeed hurt in actual usage, with locales/charsets that utilize the upper 128 values of an 8-bit characters (e.g. ISO-Latin-15).

The typical implementation of <ctype.h> stuff is as a set of macros that do their work by using the argument, plus 1, to index into a locale-specific, 257-element array.  This works because the allowed range of inputs to these functions, on such typical platforms, is EOF (== -1) to 255, inclusive.  But that means e.g. tolower('Ä')  might end up accessing array index -113 that way --- and trigger warning -Wchar-subscripts if that's enabled.

As to the DIR initializer: that's an oversight I forgot to keep out of the patch.  I was going through messages from Clang's scan-build analysis tool...

Anonymous
Fri 24 Feb 2017 02:32:47 PM UTC, comment #2: 

Ping?  Hans?

Benno Schulenberg <bens>
Group administrator
Mon 13 Feb 2017 11:09:02 AM UTC, comment #1: 

Hello Hans,

Thanks for the patch.  Hmm...  Do you know of any system where this would go wrong (without the casts)?  Because, as far as I know, the plain char on my system is a signed char, and I haven't seen any problems.

About the unrelated "DIR *dir = NULL" that is in your patch, does your compiler complain about it (when the NULL is missing)?  Because setting it to NULL shouldn't really be needed: whenever do_browser() is entered, path will be set, and so "dir = opendir(path)" will be executed.

Benno Schulenberg <bens>
Group administrator
Sun 12 Feb 2017 06:11:27 PM UTC, original submission:  

In several places, the nano source calls functions/macros from <ctype.h> with arguments of type plain (char).  That is at least formally incorrect because the argument is supposed to be an (int) from the value range of (unsigned char) --- the only negative argument allowed is <stdio.h>'s EOF.  Plain (char), OTOH, may be negative, and if it is, that causes undefined behaviour.

The correct solution is to case such arguments to (unsigned char).  Every time.

See attached git format-patch.

Anonymous

 

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

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (wishing to get feedback)
  • -email is unavailable- added by bens (Posted a comment)
  •  

    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.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-04-02 bens Open/ClosedOpen Closed
    2017-03-06 bens Severity3 - Normal 2 - Minor
        StatusNone Fixed
        SummarySlight mistreatment of &lt;ctype.h&gt; functions slight mistreatment of <ctype.h> functions
    2017-02-24 bens Carbon-Copy- Added -email is unavailable-
    2017-02-13 bens Assigned toNone bens
    2017-02-12 None Attached File- Added 0001-Make-sure-calls-to-ctype.h-functions-macros-work-eve.patch, #39740

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code