Project

General

Profile

Bug #7344

wcsncasecmp shouldn't take one for the road

Added by James Blachly about 4 years ago. Updated about 4 years ago.

Status:
Closed
Priority:
Normal
Category:
lib - userland libraries
Start date:
2016-08-31
Due date:
% Done:

100%

Estimated time:
2.00 h
Difficulty:
Medium
Tags:
Gerrit CR:

Description

wcsncasecmp(3C) should perform Wide C String N Case CoMParison between two wchar_t * (comparing at most n characters).

However, this fails on illumos in very strange ways which are C versus C++ dependent.

Strangely, defining a const char * fixes_problem = "WHATEVER" anywhere in the program seems to fix the issue ... when compiling with g++. If compiling as C code with gcc, the program still returns wrong value, but now it returns -641 (instead of 641 as below).

Demonstration: demo1 fails ; in demo2 I define a const char * and wcsncasecmp now returns the correct value (zero), but only when compiled as C++

james@smartos-dev:/tmp % cat demo1.cpp
#include<wchar.h>
#include<stdio.h>
#include<stdlib.h>

int main(void) {

    wchar_t s1[1024];
    wchar_t s2[1024];
    size_t n = 3;

    int e = mbstowcs(s1, "SET", 3);
    e = mbstowcs(s2, "SET", 3);

    int ret = wcsncasecmp(s1, s2, n);
    printf("ret: %i\n", ret);

    return 0;
}
james@smartos-dev:/tmp % cat demo2.cpp
#include<wchar.h>
#include<stdio.h>
#include<stdlib.h>

int main(void) {

    const char *fixes_problem = "SET";

    wchar_t s1[1024];
    wchar_t s2[1024];
    size_t n = 3;

    int e = mbstowcs(s1, "SET", 3);
    e = mbstowcs(s2, "SET", 3);

    int ret = wcsncasecmp(s1, s2, n);
    printf("ret: %i\n", ret);

    return 0;
}
james@smartos-dev:/tmp % diff demo1.cpp demo2.cpp
6a7,8
>     const char *fixes_problem = "SET";
>
james@smartos-dev:/tmp % ./demo1
ret: 641
james@smartos-dev:/tmp % ./demo2 
ret: 0

Now demonstrating the difference between C++ and C:

james@smartos-dev:/tmp % mv demo2.cpp demo2.c 
james@smartos-dev:/tmp % g++ demo2.c -o demo2_c++
james@smartos-dev:/tmp % ./demo2_c++
ret: 0
james@smartos-dev:/tmp % gcc demo2.c -o demo2_c 
james@smartos-dev:/tmp % ./demo2_c 
ret: -641
#1

Updated by James Blachly about 4 years ago

So, further datapoint: I've just found that for demo1, which fails with C++, it correctly returns zero if compiled with gcc.

#2

Updated by Robert Mustacchi about 4 years ago

  • Subject changed from wcsncasecmp in stdlib fails wchar_t comparisons to wcsncasecmp shouldn't take one for the road
  • Assignee set to Robert Mustacchi
  • % Done changed from 0 to 80
  • Tags deleted (needs-triage)

The problem here is that the wcsncasecmp function is slightly broken. It is using a postfix operator to check the number of characters to compare when it should be using a prefix operator. This means that we will always end up checking one additional character beyond what we were asked to, which is often garbage.

#3

Updated by Electric Monk about 4 years ago

  • Status changed from New to Closed
  • % Done changed from 80 to 100

git commit f2d34afa1058d195513e7ab9a6c1f0ce38b4d05b

commit  f2d34afa1058d195513e7ab9a6c1f0ce38b4d05b
Author: Robert Mustacchi <rm@joyent.com>
Date:   2016-09-07T23:05:51.000Z

    7350 wcsncasecmp reads data from buffers when count is zero
    7344 wcsncasecmp shouldn't take one for the road
    Reviewed by: Dan McDonald <danmcd@omniti.com>
    Reviewed by: Ryan Zezeski <ryan@zinascii.com>
    Reviewed by: James Blachly <james.blachly@gmail.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Also available in: Atom PDF