2

I do not understand the meaning if this warning I get. My code compiles but fails to read inputs half of the time.

typedef struct a
{
    double* inputs;
} A;

I create a new struct A:

A* createA(double* inputs) 
{
    A* a = (A*)malloc(sizeof(A));
    a->inputs = inputs;
}

Why do I get the 6011 warning on a->inputs = inputs;?

On the Microsoft Documentation it says to test if the given argument is NULL before feeding a->inputs, witch is indeed a good practice to track errors.

But even adding:

A* createA(double* inputs) 
{
    A* a = (A*)malloc(sizeof(A));
    if (inputs != NULL)
        a->inputs = inputs;
    else /* error */
}

I still get the warning but on the if statement.

BeGreen
  • 765
  • 1
  • 13
  • 39
  • 6
    I believe the warning is referring to the variable `a`, not `inputs`, ie. you are not handling the case when the allocation fails. But why are you using `malloc` instead of `new` in c++? – clcto Apr 15 '19 at 17:51
  • 2
    `if (inputs != NULL) a->inputs = inputs; ` - don't you want to test `a` rather than (or in addition to) `inputs`? Also; don't use `malloc` in C++. And prefer `nullptr` over `NULL`. – Jesper Juhl Apr 15 '19 at 17:52
  • @clcto I'm a beginner in c++, I mostly learned C. Old habit. I also should create a Class for A. And you are right about `a`. Thank you. – BeGreen Apr 15 '19 at 17:55

3 Answers3

2

As @clcto explained, it is because you are not doing error checking.

If you do not care about OOM, and you can use exceptions, simply do:

struct A
{
    double* inputs;
};

A* createA(double* inputs) 
{
    A* a = new A;
    a->inputs = inputs;
    return a;
}

Because new will throw on OOM. If you cannot use exceptions and/or want to check the return value instead, look into using new(std::nothrow).

Also note the notation on struct A, instead of a typedef (unneeded in C++). Also, you should probably look into using std::unique_ptr<A> instead, too.

Acorn
  • 24,970
  • 5
  • 40
  • 69
1

The problem is that malloc may return NULL when it fails. In this case, writing to a->inputs in the following line would be dereferencing the NULL pointer, which is Undefined Behavior, and should be avoided. It's admittedly unlikely that malloc will fail, but since you like to write safe code, you should be testing the return value of malloc:

A* a = (A*)malloc(sizeof(A));
if (a != NULL){
    a->inputs = inputs;
} else {
    /* Handle error */
}

Since you're writing C++ coming from a C background, you should be aware that the code you're writing is essentially still C, and is very different from modern C++, in which we do lots of things very differently. In C++, there are lots of very helpful tools and idioms we like to use.

I'm assuming that inputs is a list of numbers. In C++, I would rewrite your code as follows:

// no typedef
struct A {
    vector<double> inputs;
};

// no createA

// Example usage:
int main(){
    A a;

    // no manual memory management!!!

    // no pointers!

    // how many inputs?
    cout << "There are " << a.inputs.size() << " inputs\n";

    // add two inputs
    a.inputs.push_back(3.83);
    a.inputs.push_back(1.01);

    // print everything
    for (const auto& input : a.inputs){
        cout << input << " ";
    }

    return 0;
}

This how we usually do things in C++ nowadays. There's lots of things going on under the hood, but the overall experience is very safe and friendly once you know what you're doing. If you're interested, I would suggest that you take your pick from this list of good C++ books and read carefully. Otherwise, it might be simplest to stick to C, if that's what you're comfortable with.

alter_igel
  • 6,899
  • 3
  • 21
  • 40
-3

There is a memory problem in your case, because in the struct a, there is the pointer inputs.

inputs is not assign. You must allocate some memory a make inputs point to it OR you must assign NULL to it, in a constructor for example (yes, you can add a constructor to a struct in C++.)

PierreS
  • 1
  • 3
  • I do feed `createA` with an allocated `double*`. The problem was not checking if `a` was `NULL` at it's creation. – BeGreen Apr 15 '19 at 17:57
  • may be there is a confusion between two different inputs : the one inside the class and the other as function argument. Could you try to change one of them ? – PierreS Apr 15 '19 at 18:01
  • Thanks for contributing! I'm having some trouble understanding your answer though. This isn't a problem about what the member `input` is pointing to, but rather about dereferencing `a` after allocating it. Also, note that constructors won't be called by `malloc`, so this answer is a bit incomplete. – alter_igel Apr 15 '19 at 18:03