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

 
 

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

Submitted by:  None
Submitted on:  Sun 12 Feb 2017 06:11:27 PM UTC  
 
Severity: 2 - MinorStatus: Fixed
Assigned to: Benno Schulenberg <bens>Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

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

The changes were released in 2.8.0.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Mon 06 Mar 2017 08:18:48 PM UTC, comment #4:

Applied to git, 636b7348. Thanks.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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.)

Attach File(s):
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 02 Apr 2017 09:48:22 AM UTCbensOpen/ClosedOpen=>Closed
    Mon 06 Mar 2017 08:18:48 PM UTCbensSeverity3 - Normal=>2 - Minor
      StatusNone=>Fixed
      SummarySlight mistreatment of <ctype.h> functions=>slight mistreatment of functions
    Fri 24 Feb 2017 02:32:47 PM UTCbensCarbon-Copy-=>Added -unavailable-
    Mon 13 Feb 2017 11:09:02 AM UTCbensAssigned toNone=>bens
    Sun 12 Feb 2017 06:11:27 PM UTCNoneAttached File-=>Added 0001-Make-sure-calls-to-ctype.h-functions-macros-work-eve.patch, #39740

    Back to the top


    Powered by Savane 3.1-cleanup1