Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

delete [] after casting unsigned char* to const unsigned char*

Tags:

c++

Roughly speaking, I have a class that holds an array of const unsigned char. Objects of this class are created by a special factory function that also takes care of constructing the array (on the heap). When an object is created in the factory function, it will be given the pointer to the array. The array won't be copied, the object will just use the given pointer. On destruction, it will deallocate the chunk of memory that the array occupied.

class Foo
{
private:
    const unsigned char* array;
    size_t size;

public:
    Foo(const unsigned char* array, size_t size) : array(array), size(size) {}
    ~Foo() {delete [] array;}
};

Foo* factoryFunction(const void* data, size_t size)
{
    unsigned char* array = new unsigned char[size];
    memcpy(array, data, size);
    return new Foo(array, size);
}

Now I wonder whether there are any side effects, because new [] returns unsigned char *, but delete [] is called for const unsigned char *. I don't get any segmentation faults though.

like image 656
user2298132 Avatar asked Apr 19 '13 08:04

user2298132


3 Answers

This is fine, and non-normative text in the standard suggests the same thing:

[C++11: 5.3.5/2]: If the operand has a class type, the operand is converted to a pointer type by calling the above-mentioned conversion function, and the converted operand is used in place of the original operand for the remainder of this section. In the first alternative (delete object), the value of the operand of delete may be a null pointer value, a pointer to a non-array object created by a previous new-expression, or a pointer to a subobject (1.8) representing a base class of such an object (Clause 10). If not, the behavior is undefined. In the second alternative (delete array), the value of the operand of delete may be a null pointer value or a pointer value that resulted from a previous array new-expression. If not, the behavior is undefined. [ Note: this means that the syntax of the delete-expression must match the type of the object allocated by new, not the syntax of the new-expression. —end note ] [ Note: a pointer to a const type can be the operand of a delete-expression; it is not necessary to cast away the constness (5.2.11) of the pointer expression before it is used as the operand of the delete-expression. —end note ]


There is a possible controversy stemming from the following passages:

[C++11: 5.3.5/3]: In the first alternative (delete object), if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the behavior is undefined. In the second alternative (delete array) if the dynamic type of the object to be deleted differs from its static type, the behavior is undefined.

Though the intent of this passage appears to me only to be handling polymorphism, combined with the following passage it may be interpreted to mean that, strictly speaking, your code actually invokes undefined behaviour:

[C++11: 3.9.3/1]: [..] The cv-qualified or cv-unqualified versions of a type are distinct types; [..]

However, I'm reasonably confident that this can be treated as a wording defect in the standard; the intent seems clear to me.


Likewise, this rule could even suggest that the program will not compile:

[C++11: 12.5.4]: [..] If a delete-expression begins with a unary :: operator, the deallocation function’s name is looked up in global scope. Otherwise, if the delete-expression is used to deallocate a class object whose static type has a virtual destructor, the deallocation function is the one selected at the point of definition of the dynamic type’s virtual destructor (12.4). Otherwise, if the delete-expression is used to deallocate an object of class T or array thereof, the static and dynamic types of the object shall be identical and the deallocation function’s name is looked up in the scope of T. If this lookup fails to find the name, the name is looked up in the global scope. If the result of the lookup is ambiguous or inaccessible, or if the lookup selects a placement deallocation function, the program is ill-formed.

But again, this is not the intent of the rule, which addresses polymorphism in the absence of a virtual destructor, and genuine ambiguities across multiple class declarations.


To sum up, your best bet when interpreting the wording of these rules is still on the non-normative but very clear note we started with.

like image 123
Lightness Races in Orbit Avatar answered Oct 28 '22 22:10

Lightness Races in Orbit


Shouldn't be any problem since "const"-ness on "unsigned char"(or any datatype) makes it read-only data.

Using "[]" in delete tells that an array has to be deleted.

To understand what goes behind "delete" vs "delete []", read this.

like image 1
Arun Avatar answered Oct 28 '22 21:10

Arun


First of all you should avoid using this kind of arrays in c++ you should use std::vector<unsigned char> instead.

about the const keyword. It just tells the compiler and the one who reads the code that this member/variable/parameter should not be changed.

With pointers there the position of the const keyword is important:

(EDIT: found the answer of Greyson where i borrowed this part)
The const keyword marks the part on its left as constant (if it is at the beginning it is for the type right after it so const unsigned char and unsigned char const are equal)

To mark the pointer as const you do this (content is still changeable):

unsigned char * const aConstantPointerToAMutableContent;

To mark the content as const you will do this:

const unsigned char * aPointerToAConstantContentA;
unsigned char const * aPointerToAConstantContentB;

To mark both as constant:

const unsigned char * const aConstantPointerToAConstantContent;

It is a hint for the compiler and for the user so that it is clear what is done or not done with the data. if the parameter is const unsigned char * then the user would know that this content would not be changed if it is passed.

because the const keyword is only a mark if something is changeable it has no effect to the size itself, so for the delete it should no have an effect. (see answers and comments of Lightness Races in Orbit and also "Is const_cast safe?" could be for interrest ).

But what i don't like about your code is that the constructor is public. I could call it directly, but because the parameter is const unsigned char* array i would not expect that the class would change the content or that it is deleted on destruction. (i would not expect that directly creating the object would behave in a different way to using the factory.)

So i would make the factory method as a staticmethod of the class and make the constructor protected.

like image 1
t.niese Avatar answered Oct 28 '22 23:10

t.niese