What is wrong with the following strcmp() code?

Question

I have a multidimensional array of a structure, whose content is initialized to '\0' initially and later occasionally. After some values are inserted, it is sorted on fing_print in an ascending order. Then the first n entries of the sorted array is copied into another array. In my implementation some of the array elements might not be assigned values, so they contain the '\0' values assigned to them earlier and they need to be ignored when copying into the other array.

My problem is, the code breaks at "__DEFINE_CPP_OVERLOAD_SECURE_FUNC_0_1( errno_t, strcpy_s, _Post_z_ char, _Destination, _In_z_ char const*, _Source ).

And upon debugging I have noticed that it breaks at the strcpy_s(selected[j][r].fing_print, hash_table[j][i].fing_print); line when only one of the j (BUCKETS) contains data.

for (int j = 0; j < BUCKETS; ++j) {
    // select rep fps here according to select size

    for (int i = 0,r=0; i<B_ENTRIES, r < SELECT_SIZE; ++i) {
        if (!strcmp(hash_table[j][i].fing_print, garb)) {
            // garb contains '\0'
            continue;
        }
        prev = i-1;
        if (!strcmp(hash_table[j]i].fing_print,
                    hash_table[j][prev].fing_print)) {
            // make sure the same fingerprint is not selected twice
            continue;
        }
        strcpy_s(selected[j][r].fing_print,
                 hash_table[j][i].fing_print);
        ++r;    
    }
}

Also see here: https://msdn.microsoft.com/en-us/library/032xwy55.aspx — alk, Sep 30 '17 at 08:44
Your `for` loop exit test is a comma expression that makes no sense. Since there are no side effects in the first part, it serves no purpose. I.e. `i — Tom Karzes, Sep 30 '17 at 09:04
@alk I did as you said and I got thousands of errors from a code which compiled with no errors earlier. I hope you can tell me why. — George, Sep 30 '17 at 09:28
@TomKarzes i am learning while making those mistakes and corrected by people like you. But I didn't mean that I have not read about it before I started on my project. You just can't learn everything the first time. — George, Sep 30 '17 at 09:31
Strcpy_s has 3 parameters - destination, size and source. You only have 2 so it is taking your source as the size and setting the source as 0. Possibly the reason for your original crash — cup, Sep 30 '17 at 09:46
the question is a bout a run time problem. However, only a small snippet of the code is posted. How are we to debug without code the cleanly compiles, is short, and still exhibits the problem? please post a [mcve] — user3629249, Sep 30 '17 at 16:32
`strcpy_s` in C library has **three** parameters. You are calling it with **two** arguments. Your code will not compile in a C compiler in Visual Studio. You are making something up. — AnT stands with Russia, Oct 02 '17 at 06:28
There's a templatized version of `strcpy_s` that deduces the size from the array being passed. Without seeing the appropriate declarations though it's hard to know if it's deducing the right value or not. https://msdn.microsoft.com/en-us/library/td1esda9.aspx — Retired Ninja, Oct 02 '17 at 06:46
Note that `strcpy_s()` only needs two arguments when compiling C++ code if the first argument's type is an actual array (instead of a pointer). The library uses template type deduction to determine the size of the destination in that case. — Michael Burr, Oct 02 '17 at 06:48

George · Accepted Answer · 2017-10-02T06:33:53.363

As stated in the comment section my mistake was in using the comma in the loop exit conditions for (int i = 0,r=0; i<B_ENTRIES, r < SELECT_SIZE; ++i) .correcting it to the following worked fine.

for (int j = 0; j < BUCKETS; ++j) {
    // select rep fps here according to select size
for (int i = 0,r=0; i<B_ENTRIES && r < SELECT_SIZE; ++i) {
    if (!strcmp(hash_table[j][i].fing_print, garb)) {
        // garb contains '\0'
        continue;
    }
    prev = i-1;
    if (!strcmp(hash_table[j]i].fing_print,
                hash_table[j][prev].fing_print)) {
        // make sure the same fingerprint is not selected twice
        continue;
    }
    strcpy_s(selected[j][r].fing_print,
             hash_table[j][i].fing_print);
    ++r;    
  }
}

The strcpy_s() function worked fine as it is, but again as pointed out in the comments above it missed one parameter which is the size of the destination array and so should be corrected to

strcpy_s(selected[j][r].fing_print,sizeof(selected[j][r].fing_print),
             hash_table[j][i].fing_print);

What is wrong with the following strcmp() code?

1 Answers1