Cc65: [c64] tolower() and toupper() are broken

[doj]

on the c64 target to tolower() and toupper() functions are broken. several characters are not correctly converted. Also some punctuation characters are converted, while they should not be changed by either tolower or toupper. The same goes for the graphic characters.

I have a sample program which prints several characters and allows for key presses, see https://gist.github.com/doj/74c11566bbea909bd28e61ea05016efd

See also the following screenshot of the program.

[doj]

I printed the _ctype table, which is used by tolower() and toupper() to classify a character. It seems it is not initialized correctly, as the values don't make sense.

[oliverschmidt]

First of all thanks for the excellent bug report :-)

As it turned out the issue wasn't specific to the C64 at all. Rather tolower() and toupper() were foobared on all targets since two months.

Fixed with https://github.com/cc65/cc65/commit/df015f47662d3b2f44a383bd5dc520216c9285f7

[polluks]

I suggest a unit test for this case.

[oliverschmidt]

I thought the same at first sight... However, so far we have about no tests for the C library, only for the compiler (and its runtime). I think we have a much better understanding of the C library code than we have of the compiler code. Therefore I think tests for the compiler are much, much more important than tests for the C library. We didn't have significant regressions in the C library since we're on GitHub. I think tolower()/toupper() are the worst ones - or I forgot about the other ones ;-)

From that perspective I'm wondering if really makes sense to start out with tests (only) for tolower()/toupper() just because they happened to be the victims. They are not at higher risk for further regressions than any other (untested) part of the C library. If I had to choose I'd rather start with C library tests for high risk functions, e.g. scanf()/printf().

Just my two cents, Oliver

[Marrin]

This reads a little like no tests unless stuff that is at higher risk to break has tests. If someone wants to contribute tests for a regression, or any test, I don't think they should be discouraged. I would even say it would be nice to ask (not demand of course) for tests coming with all but very trivial bug fixes.

[oliverschmidt]

Hm, let's get some things straight here:

• I'm not against tests. I was the one who made contiguous integration tests for cc65 a reality in the first place.

• Regression tests don't do wonders. The regression at hand is a great example. The authors of the PR in question just forgot about tolower()/toupper(). If his PR would have come with tests, there would have been no test for those two functions. The regression would have happened anyhow.

• On the other hand regressions in the C library tend to be rather easily fixed, see this PR.

• In general the cc65 community has more members who are good at telling others what to do then members who are good at doing things.

I will _NOT_ urge any C library contributer to provide tests. The risk of further reducing the number of contributions is _WAY_ higher than the risk of regression!

<topic closed>