[PATCH] scanner.c: prefer strchr() over for loop and toupper() in place

Thiago Macieira thiago at kde.org
Tue Jan 9 18:14:07 UTC 2024


On Wednesday, 3 January 2024 02:48:26 -03 James Tirta Halim wrote:
> -       u = xstrdup(src);
> -       for (i = 0; u[i]; i++)
> -               u[i] = toupper(u[i]);
> -       u[i] = '\0';
> +       dst = u = fail_on_null(malloc(strlen(src) + 1));

This does have the advantage of not writing to every byte twice, just once.

However, the big elephant in the room is that:

> +       while ((*dst++ = toupper(*src++)));

You're violating the rule of use of toupper/tolower: NEVER. Those functions 
have a design flaw that they operate on bytes, not on full strings. Uppercasing 
and lowercasing need to see more than one byte to be accomplished correctly.

Moreover, those two functions are locale-dependent, so the output of this 
function is also locale-dependent, something that the scanner probably 
shouldn't be. Try testing the tool with an 8-bit Turkish locale to see what 
happens.

I suggest you ditch toupper() in the first place and just do the ASCII 
uppercasing manually.

PS: I suggest either moving the ; to the next line or using brackets to make 
it evidently clear that you intended an empty while loop.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel DCAI Cloud Engineering





More information about the wayland-devel mailing list