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

Reply via email to