[pacman-dev] [PATCH] mbscasecmp fails in 1% of time.

[Andrew Gregory]

On 06/10/16 at 06:40pm, Tobias Stoeckmann wrote:
> The width of wchar_t is allowed to be of the same width as long,
> according to standards. The return type of mbscasecmp is int though.
> 
> On amd64 with a 32 bit int, this means that mbscasecmp can return
> zero (indicating that strings are equal) even though the input
> strings differ.
> 
> No known system exists that could trigger this.
> 
> Signed-off-by: Tobias Stoeckmann <tobias at stoeckmann.org>
> ---
> There is a reason that I picked this one, annotation will help.
> ---
>  src/pacman/util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 81780f7..e38ef06 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -1503,7 +1503,7 @@ int select_question(int count)
>  	return (preset - 1);
>  }
>  
> -static int mbscasecmp(const char *s1, const char *s2)
> +static wchar_t mbscasecmp(const char *s1, const char *s2)

Good catch, but the solution needs work.  wchar_t can be unsigned.
While that's fine for how this function is currently used (only
testing for equality), it no longer works as a three-way comparison
function as its name suggests.  The return type should stay 'int' but
the return value should either be normalized to an int or changed to
a simple equality test with an appropriate change to the function
name.

And please make the first line summary of your commit message more
descriptive of what the patch actually does.  "X fails sometimes" is
not particularly helpful when looking through the log.

>  {
>  	size_t len1 = strlen(s1), len2 = strlen(s2);
>  	wchar_t c1, c2;
> -- 
> 2.8.4