Update of patch #1582 (project freeciv):
Status: Ready For Test => None
Follow-up Comment #5:
> ATM there is an issue with MAX_LEN_NAME. This is supposed to
> limit city names, for instance, at 32 characters. Instead it
> limits then at 32 bytes which is incorrect. Most places which
> use it should instead use a longer or malloc'd buffer, and use
> fc_utf8_strlen() to make sure the name isn't too long.
A problem would be for the packets. The packet size seems to need a byte
limit, not in characters.
> How do we know just because a character is 4 bytes long that
> bytes 2-4 are valid? Does UTF-8 allow any byte sequence in
> here? What about malicious chats from the client?
It is the role of base_fc_utf8_char_validate() called by
fc_utf8_char_validate() and fc_utf8_validate(), then
> Casts should not be needed in passing values to these
> functions. Doing so is a sign the parameters used are not
> right. In fact I think one such case is quite wrong.
I am not sure to understand what do you criticize here. The only casts are to
unsure the calculation is done with unsigned characters. There are also some
casts to (char *) which are used to do internally tricky things (e.g. in
> Each function provided should do everything that it needs to.
> Specifically, fc_utf8_char_size is not a useful function in
> its current form (except for internal usage).
I agree, this function was needed for defining fc_utf8_char_next() in the
first patch, I forgot to remove it from the header file in the second.
> Are we sure that using these variants everywhere is easier
> than converting all freeciv to wide characters?
I don't know.
Reply to this item at:
Message posté via/par Gna!
Freeciv-dev mailing list