std::isspace requires unsigned about convert HOT 22 CLOSED

http://en.cppreference.com/w/cpp/string/byte/isspace

The behavior is undefined if the value of ch is not representable as unsigned char and is not equal to EOF.

https://linux.die.net/man/3/isspace

These functions check whether c, which must have the value of an unsigned char or EOF,

from convert.

Much appreciated. I'll have it addressed ASAP.

from convert.

Unm, I presume is MS Visual Studio. What version is it? The standard POSIX signature of the function is
int isspace(int c)
It is defined in
#include <ctype.h>
via the cctype header.

Seems like MS have it messed it up. What am I missing?

from convert.

1. We get a compilation warning "unsigned char is required in call to 'isspace". I believe it is wrong for the compiler to issue such a warning because "int is actually required by isspace". So, technically, 'char' is fine (auto-promoted to int).

2. The statement "behavior is undefined if the value of ch is not representable as unsigned char" describes the internal run-time function behavior. However, it does not expect "unsigned char" as the compiler claims. For example, EOF is not "unsigned char". It means that if the supplied "int" goes out of [-1,256) range, then "behavior is undefined". Consequently, I expect

char ch;
isspace(ch);

to be a valid piece of code at compile-time because it matches the isspace(int) signature (char is promoted to int).

At run-time it is valid as long as 'ch' is ASCII [0,127].

The problem will arise if, say, ch=-55. However, the compiler cannot pick it up. It'll be an internal run-time "undefined behavior".

Do you agree?

from convert.

The error is from a code analyzer, not from the compiler. But yeah, negative values will cause undefined behavior.

The input doesn't have to be unsigned char but it has to be in -1 .. 255 (inclusive)

from convert.

Uh, a code analyzer. Fair enough. So, the compiler is innocent then. Good. :-) So, technically, compilation-wise the code is correct and there seem nothing to fix. Then, yes, the code analyzer warns us of the std::isspace() function vulnerability... which is fair enough too. I suspect the necessary validation checks are outside boost::convert scope. Do you agree?

from convert.

No, undefined behavior here is unacceptable.
I don't know the right solution, I've used v & 0xff myself. A static_cast might work as well, perhaps you should ask on the mailing list.

from convert.

Correction: there is no undefined behavior here, i.e. in boost::convert. The undefined behavior is in isspace(). Now you are essentially saying that if the user (on one side) messed it up and isspace() (on the other side) does not like it, then it is the middle-man (boost::convert) responsibility to clean it all up. I am not sure I am prepared to accept that. I'll have to think about it. Thanks for the input all the same.

from convert.

Correction: there is no undefined behavior here, i.e. in boost::convert.

How so?
Does boost::convert have preconditions that forbid input having 'negative' chars?

from convert.

There are 3 participants -- the caller, boost::convert, isspace(). It is the isspace() that introduces the undef. behavior but does not addresses that. Still, you seem you believe it's boost::convert task to address that. I am not sure I am that definitive about that. :-)

from convert.

No, it is convert that violates preconditions of isspace..

from convert.

isspace does not have preconditions to violate. Consequently, boost::convert does not "have preconditions that forbid input having 'negative' chars" (quoting you). Just like isspace(). If instead, you insist that isspace() does have preconditions, then so boost::convert. In other words, boost::convert has (or does not have) preconditions just as isspace. So, if isspace does have those, then it is the user who violates those. Isn't it? Or no matter what it is always boost::convert's fault? :-)

from convert.

Yes ;)

In other words, boost::convert has (or does not have) preconditions just as isspace.

OK, so where are those preconditions of boost::convert documented?

from convert.

from convert.

After Peter Dimov's explanations I understand that it is indeed a problem. Corrected. Thank you for your help. Much appreciated.

from convert.

Boost 1.64 does not contain this fix, does it? :(

from convert.

Unfortunately it is correct. In late March 1.64 was already closed for changes (or so I understood). So, I did not merge the changes into the master branch. 1.65 is now open, so I did it now. I am sorry.

from convert.

1.64 was open for fixes after March 24th.. this should've been included.

from convert.

By Rene Rivera 09MAR17: "The master branch is now closed for all changes except by permission for the Beta release".

If 1.64 was, in fact, open as you indicated, then I misinterpreted that statement above. My most humble apologies.

from convert.

On 24-03-17 02:00, Rene Rivera via Boost wrote:
The master branch is is now open for post-beta merges, but only as
described in the Post-Beta Merge Policy. See <
https://github.com/boostorg/boost/wiki/Release-Beta-Merge-Policy>

from convert.

Just to verify, is it in master yet?

from convert.

Yes, the develop branch has been merged into the master some time ago. Unfortunately, it was late for 1.64 as we discussed.

from convert.