in the following function: static PRUnichar _ToLower(PRUnichar aChar) it calls CheckCaseConversion() where an assertion can be triggered, but that the error is not passed back to _ToLower(). This is causing a crash in the line right after CheckCaseConversion() where it tried to dereference a null pointer.
the function is in the file mozilla/xpcom/ds/nsCRT.cpp
What is the desired behavior of case conversion, if no case converter could be loaded, since it has no ability to return an error code? The code seems to indicate that returning an unconverted character, with no indication that the method failed, is the right thing. I don't think it is. If the logic calls for lower case, and the function is not available, then to me it seems that the program cannot function. There is no documentation that I can see that _ToLower is permitted to return upper case if it isn't in a proper state to return lower case.
I agree that it should not alter the case of the character if the converter could not be loaded. Since _ToLower() is not documented to return error values, I would be okay if it just returned the character unchanged. This would be better than the crash :)
I need to understand why a crash is not better. If there are important operations relying on the capability of case conversion that will silently malfunction, I think a crash is better than returning the original character, which is the wrong answer. The function is not documented as returning the right answer if it feels like it. To me, it seems like the real problem here is that whatever function relies on it shouldn't be invoked when it is not available.
I need more information here. Why is it valid for ToLower to silently fail? Is it not better to fix the problem of the proper component not being loaded yet, and perhaps make the failure more explicit?
The problem was that the file that the case converter is in was missing. This part has been fixed. I just thought that the crash resulting from this missing file was a bug. It is not correct for ToLower to fail silently. I was thinking that it would be better than a crash since there's not a way to return an error. I agree that a more explicit failure needs to be applied here, but I am not sure as to what the proper procedure is. This is not a priority for me at all. The crash resulting from this bug has been fixed for me, but the crash can still happen for someone else.
spam: Adding crash keyword...
This works about the way I would expect it to. The bug is that it is being called in a situation where it cannot give a valid defined result. I guess an alternative would be to return -1. Is that acceptable?