C++: Issue with clearing a vector of objects

Question

EDIT: It seems this is a problem with using memset on my struct, instead clearing the vector. Thanks to all that have provided advice!

I'm attempting to clear my vector of Subject objects (my own defined class) called people. The vector sits in a struct (pQA) and is defined as the following:

typedef struct _FSTRUCT_
{
    const char * filePath;
    std::vector<Subject> people;
    long srcImageWidth;
    long srcImageHeight;
    STRUCT_CONFIG_PARAMS * configParam;
    unsigned char * imageBuf;
    int imageBufLen;
} STRUCT_FSTRUCT;

I am creating the pQA struct by:

STRUCT_FSTRUCT *pQA = NULL;
pQA = new STRUCT_FSTRUCT();
memset(pQA,0,sizeof(STRUCT_FSTRUCT));

I populate 'people' with data by using the Subject class' set methods. This is all fine. What I am wanting to do is then reset 'people', i.e. clear out all data and set the size to 0. I call the below method:

int ResetFaceCollection()
{
    if (!pQA->people.empty())
    {
        pQA->people.clear();
    }
}

The clear() line throws a debug assertion failed error message which states "Expression: vector iterators incompatible".

I'm not sure if this has anything to do with Subject's destructor:

Subject::~Subject(void)
{
}

I'm not using any pointers, so from what I've gathered, the destructor looks OK. I have, of course, defined the destructor in my .h file also ~Subject(void);.

I'm a bit lost as to why this is happening. Can anyone provide some insight?

I apologize if I'm omitted any necessary code, can update upon request!

why use a weirdly named struct with only a vector inside? Just use the vector directly. Also nothing that seems wrong in the code you showed, you might want to post a bit more. **Edit:** Oh and checking if it's empty is superfluous, `.clear()` is a noop anyways if it's empty so just call it without that if. — AliciaBytes, Nov 18 '14 at 09:34
Likely: `pQA` isn't a valid pointer. (The `typedef struct X...` roundabout is a C-ism which is completely unnecessary in C++.) — molbdnilo, Nov 18 '14 at 09:38
@LBran That doesn't indicate that `pQA` was valid, only that it points to writable memory. How are you creating and using `pQA` before this point? — molbdnilo, Nov 18 '14 at 09:49
@molbdnilo - I am creating pQA by: `STRUCT_SPID_QA *pQA = NULL; pQA = new STRUCT_SPID_QA(); memset(pQA,0,sizeof(STRUCT_SPID_QA));` — LKB, Nov 18 '14 at 09:51
Do not use memset on C++ classes (or structs which are classes with public members); read about constructors and destructors instead. For instance, you just zeroed all of a std::vector's bookkeeping information. While that may (or may not, I don't know!) just coincidentally be the data that is in an empty vector -- after all, the length is zero and the pointer value is irrelevant --, it is undefined behaviour. Your C background is appreciated but anti-paradigmatic in C++. — Peter - Reinstate Monica, Nov 18 '14 at 09:56
@LBran Forget everything about C programming *immediately* and get a decent book about C++. — molbdnilo, Nov 18 '14 at 09:58
@molbdnilo Honestly, it's not my choice to use C. I have to use these calls. :/ — LKB, Nov 18 '14 at 10:01
@LBran You should mention the `memset` call in an edit to your question as it is the reason things are going wrong. Not everyone is going to read this far down the comments. — sjdowling, Nov 18 '14 at 10:01
@sjdowling How is `memset` the reason why things are going wrong? Can you please post an answer? — LKB, Nov 18 '14 at 10:02
@LBran If you must use C, then you should write C code, not C++. — molbdnilo, Nov 18 '14 at 10:09
@LBran the arguments to `memset` are 1. starting address of the memory to set, 2. value to set each byte to, 3. amount of bytes to set. so `memset(pQA,0,sizeof(STRUCT_SPID_QA))` sets every byte inside your `pQA` variable to 0, including vectors internal bookkeeping of its elements. — AliciaBytes, Nov 18 '14 at 10:14
See http://stackoverflow.com/questions/2481654/memset-for-initialization-in-c — juanchopanza, Nov 18 '14 at 10:15
Thank you very much, guys! Will make some changes now and post back with results. — LKB, Nov 18 '14 at 10:16

milleniumbug · Accepted Answer · 2014-11-18T10:20:34.760

Your std::memset call is (a) redundant, as

pQA = new STRUCT_SPID_QA(); // <---- note the parens

value-initializes the object, which initializes integers to 0 and pointers to nullptr here.

and (b) actually very wrong:

If the object is not trivially-copyable (e.g., scalar, array, or a C-compatible struct), the behavior is undefined.

Source

Your _SPID_FQA_ contains non trivially copyable object of type std::vector<Subject>, which makes _SPID_FQA_ non trivially copyable.

Rupesh Yadav. · Answer 2 · 2014-11-18T10:24:59.710

1

Note: firtly OPs didn't showed that he is using memset some where in his code that's y i gave this answer, as i thought this weired behavior is because of some problem in clear as mentioned in below links.

1) cppreference.com: says that it Invalidates any references, pointers, or iterators referring to contained elements. May invalidate any past-the-end iterators. Leaves the capacity() of the vector unchanged.

2) cplusplus.com says that : A reallocation is not guaranteed to happen, and the vector capacity is not guaranteed to change due to calling this function. A typical alternative that forces a reallocation is to use swap :

vector<T>().swap(x); // clear x reallocating

but you can use this also:

int ResetFaceCollection()
{
    if (!pQA->people.empty())
    {
        pQA->people.erase(pQA->people.begin(),pQA->people.end());
    }
}

And check if it is giving any error?

here is the probably same environment and working fine with g++, clang, VC++ link

edited Nov 18 '14 at 10:24

answered Nov 18 '14 at 09:49

Rupesh Yadav.

894
2
10
24

Thanks rupesh for your answer. The `erase()` call has the same outcome as `clear()` - debug assertion raised. The `swap()` does in fact work, but it seems a bit.. hacky? – LKB Nov 18 '14 at 09:52
yaa it is, but `erase` and `clear` should work though? – Rupesh Yadav. Nov 18 '14 at 09:54
`clear()` does the same thing as `erase()`. Calling these methods gives me the error. – LKB Nov 18 '14 at 09:56
1

@LBran That is because your code is broken because you use `memset`. You need to fix that first. Anything else is cosmetic, unrelated to the failure, and futile. – juanchopanza Nov 18 '14 at 10:12
Thanks, @juanchopanza. A few people have said that now but no explanation given. Can you please expand? – LKB Nov 18 '14 at 10:13
wooh.. you should have mentioned that memset thing brefore it self, i would have not done research on `clear` and `erase` :P – Rupesh Yadav. Nov 18 '14 at 10:20

C++: Issue with clearing a vector of objects

2 Answers2