VS2019: [C6386][C6001] warnings on modified counting sort

Question

#include <iostream>
#include <string>
#include <vector>
using namespace std;

void MCounting_Sort(vector<int>& A)
{
const int size = A.size();

int min = A[0];
for (int i = 1; i < size; i++)
    if (A[i] < min)
        min = A[i];

for (int i = 0; i < size; i++)
    A[i] = A[i] - min;

int max = A[0];
for (int i = 1; i < size; i++)
    if (A[i] > max)
        max = A[i];

int* C = new int[max + 1]{ 0 };
for (int i = 0; i < size; i++)
    C[A[i]] = C[A[i]] + 1;

int* B = new int[size];
int pos = 0;
for (int i = 0; i < max + 1; i++)
    if (C[i] > 0)
        for (int j = 0; j < C[i]; j++)
            B[pos++] = i;   // <-- C6386 Buffer overrun...
        
for (int i = 0; i < size; i++)
    A[i] = B[i] + min;   // <-- C6001 Using uninitialized memory "B"

delete[] B;
delete[] C;
}

int main()
{
vector<int> A {7,2,18,5};
for(unsigned int i=0; i<A.size(); i++) cout<<A[i]<<" ";
cout<<endl;
MCounting_Sort(A);
for(unsigned int i=0; i<A.size(); i++) cout<<A[i]<<" ";
}

output:

7 2 18 5 
2 5 7 18

The sorting algorithm works (also for negative numbers), but I get these two warnings, I'm not sure if I can fix them, because the algorithm itself probably isn't good, but it's a school assignment. Maybe I did write something wrong though.

You allocate `int* B = new int[size];`, but never initialize these values. Any particular reason why you don't use a `std::vector` for `B` as well? — πάντα ῥεῖ, Mar 08 '21 at 16:08
For the 6001 warning, you are looping from 0 to `size`, however, `B` has only been initialized from 0 to `pos`, which is not guaranteed to be greater than or equal size. This has a similar effect for the 6386 warning. — ChrisMM, Mar 08 '21 at 16:08
Don't use owning raw pointers. Replace `B` and `C` with `std::vector`. Side effect: in a debug build you will get bounds checks at runtime. — Werner Henze, Mar 08 '21 at 16:11
The reason I don't use vector is because the assignment says I need to create "arrays" of certain size and I don't know how to do that with vectors. — GeoCap, Mar 09 '21 at 12:52

Adrian Mole · Answer 1 · 2021-03-08T16:38:49.623

As others have said in the comments to your question, your code would be far better if you avoided using 'raw' pointers and made use of the containers provided by the STL.

However, as it happens, the two warnings that the MSVC gives can be readily addressed by a 'trivial' modification to a single line of your code.

You can simply add an initializer list to your allocation of the B data (as you have done for C) and ensure that you allocate at least 2 integers in that array. The latter would normally be achieved using an expression like std::max(size, 2) inside the [] of the new call; however, as you have defined a local variable called max, you cannot use that function.

Instead, you can just change this line:

int* B = new int[size];

to this:

int* B = new int[size < 2 ? 2 : size] { 0 }; // Ensure at least 2 elements and initialize

As a side note, you may like to read this: Why is "using namespace std;" considered bad practice? Because, if you replace your using namespace std; line with individual using statements for only those STL elements that you actually use, then you can have both a local variable called max and use the std::max(size, 2) expression in your new call.

The following three lines will serve as a suitable replacement:

using std::vector;
using std::cout;
using std::endl;

Or, if your compiler conforms to the C++17 (or a later) Standard, you can put all three in one statement:

using std::vector, std::cout, std::endl;

VS2019: [C6386][C6001] warnings on modified counting sort

1 Answers1