Incidencia #45034

strcasecmp() used instead of fc_strcasecmp()

Abrir Fecha: 2022-07-06 15:40 Última actualización: 2022-11-13 10:06

Informador:
Propietario:
Tipo:
Estado:
Cerrado
Componente:
Prioridad:
5 - Medium
Gravedad:
5 - Medium
Resolución:
Fixed
Fichero:
4

Details

grep -r "strcasecmp" . | grep -v "fc_strcasecmp"

Ticket History (3/15 Histories)

2022-07-06 15:40 Updated by: cazfi
  • New Ticket "strcasecmp() used instead of fc_strcasecmp()" created
2022-07-19 14:12 Updated by: cazfi
2022-07-20 09:06 Updated by: dark-ether
Comentario

i am not familiar with this specific function, what are the differences to the normal strcasecmp? if the arguments are in the same order i probably could craft a sed command to change all offending files

2022-07-20 16:03 Updated by: cazfi
Comentario

Reply To dark-ether

what are the differences to the normal strcasecmp?

Most fc_...() replacements have originally been introduced for being more portable (often just wrappers for the "real" function of the platform), or more secure (hardened).

This specific one has since been converted to use icu -> it's utf8 aware.

2022-07-21 05:28 Updated by: dark-ether
Comentario

Reply To cazfi

Reply To dark-ether

what are the differences to the normal strcasecmp?

Most fc_...() replacements have originally been introduced for being more portable (often just wrappers for the "real" function of the platform), or more secure (hardened). This specific one has since been converted to use icu -> it's utf8 aware.

i looked at how fc_strcasecmp and strcasecmp were called and they appeared to be equal so i just used ripgrep and sed to find and modify the uses.

2022-07-21 06:47 Updated by: cazfi
Comentario

I think we would want to change the cvercmp ones too despite it having an upstream separate from freeciv - the way it's part of freeciv build process, linked in statically, and uses it with freeciv provided strings, assumes such compatibility.

Can you also provide patch for all branches where this is expected (S3_0, S3_1, master)? If you have branches set up as separate git worktrees it should be quite trivial.

2022-08-05 09:01 Updated by: cazfi
2022-08-23 23:12 Updated by: cazfi
Comentario

Reply To cazfi

I think we would want to change the cvercmp ones too despite it having an upstream separate from freeciv - the way it's part of freeciv build process, linked in statically, and uses it with freeciv provided strings, assumes such compatibility. Can you also provide patch for all branches where this is expected (S3_0, S3_1, master)?

Ping

2022-10-07 09:31 Updated by: cazfi
2022-11-05 12:21 Updated by: cazfi
  • Propietario Update from (Ninguno) to cazfi
  • Resolución Update from Ninguno to Accepted
Comentario

Reply To cazfi

I think we would want to change the cvercmp ones too despite it having an upstream separate from freeciv

Attached versions have that change, and porting S3_0 required also some other changes.

2022-11-13 10:06 Updated by: cazfi
  • Estado Update from Open to Cerrado
  • Resolución Update from Accepted to Fixed

Editar

Please login to add comment to this ticket » Entrar