compiler : visual c++ 2005
os : win7 64bits

The more I study about our source codes, the more dense fogs surround my head.
According to what I know, memset can't be used to initialize POD
This is the snippet of our codes(The problem of encapsulation?
there are too many public or protected data members in our codes without any
special reason--except of "this is convenience", I am tired to refine this).

//non POD type
struct MessageQueryTester 
{
	public:
		void querySupportModel() {}
		unsigned int supportedModelSize() { return 0; }
		const char* supportedModel(unsigned int index) { return "keke"; }
		void retrieveModelDetail(const char* model_name) {}

	public:
		/// the index of received message
		unsigned int msg_index_;

		/// the illegal args size error of constructor
		bool constructor_args_size_error_;

		/// the vendor name
		boost::shared_ptr<std::string> vendor_name_;

		/// the input argument position of vendor name of constructor
		enum { VENDOR_ARG_POSITION };

		/// the input argument position of xpath of constructor
		enum { XPATH_ARG_POSITION = 1 };	

		/// the minimum input argument size of constructor
		enum { MINIMUM_ARG_SIZE = 2 };		

		/// the character array size of result value
		enum { RESULT_STR_LENGTH = 50000 };	

		/// the return value of cslim tester
		char result[RESULT_STR_LENGTH];

	private:
		/// the supported model of request vendor
		vector<boost::shared_ptr<std::string> > support_models_;
};
//create and initialize
void* initialize()
{
  MessageQueryTester* self = (MessageQueryTester*)malloc(sizeof(MessageQueryTester));
    memset(self, 0, sizeof(MessageQueryTester));
 
 return self;
}

Why using malloc and void*?The answers I got
"because the other do it like this"

But this codes already lingering about one year and nothing happen(?),
I thought this may lead to undefined behavior if I use memset on non POD type.

Thanks

malloc would allocate a raw memeory block for us,
but using memset to initialize all of the data?
what is the meaning of setting all of the memory block of

vector<boost::shared_ptr<std::string> > support_models_;
boost::shared_ptr<std::string> vendor_name_;

to 0?
I don't know this kind of solutions are safe or not,since I rarely see it.

Thank you very much

Edited 4 Years Ago by stereomatching: n/a

After some test, this way is very dangerous
If you change the data member from

vector<boost::shared_ptr<std::string> > support_models_;

to

list<int> support_models_;

The program would crash. Our program don't have any problem up until now(?),
maybe that is because the memory of the data members of this struct are
continuous and memset haven't write data to those data they should not.

In C++ you allocate memory with new and the initialization takes place in the constructor. You do not use malloc and free as the proper initialization and cleanup routines will not be invoked.

If you are creating a non-dynamic structure then the constructor is invoked at object instantiation and you can be assured that any initialization takes place before you use the object.

In C++ you allocate memory with new and the initialization takes place in the constructor.

That is what I learned from book(c++ primer 4edition and other).
I may use malloc if I need to design my own allocator but not for
general case.There are too many weird codes in our code base. Now I could
realize why there are so many languages would prefer to seal the power of
the programmers.

Edited 4 Years Ago by stereomatching: n/a

Of course, you can use malloc() but you have to be aware that it doesn't call the constructor, so, you have to do that yourself directly afterwards, through a placement-new operator.

The "initialize" function that you posted:

//create and initialize
void* initialize()
{
  MessageQueryTester* self = (MessageQueryTester*)malloc(sizeof(MessageQueryTester));
    memset(self, 0, sizeof(MessageQueryTester));
 
 return self;
}

is a repugnant piece of junk code.

You ask, is this safe? Hell no! You proved that yourself by causing it to crash when changing the container to a list instead of a vector. My guess is that setting all bytes to 0 of a vector object does not cause a crash, but that's just luck (probably, a vector only stores a pointer to the data and a size-value, and if you set both to zero it is an OK state for the object). And that is exactly how that code is expected to function: cause a later crash in most cases, go unnoticed on a few lucky(?) occasions.

There are two terrible errors in the code, (1) it uses memset to initialize the memory of the object which is completely wrong, and (2) it strips the type information out of the pointer it returns (and does a cast on that occasion, which has undefined behavior).

There are two correct options:
1) Use malloc and placement-new:

//create and initialize
MessageQueryTester* initialize()
{
  MessageQueryTester* self = (MessageQueryTester*)malloc(sizeof(MessageQueryTester));
  new(self) MessageQueryTester();
  
  return self;
};

2) Just use the regular new operator:

//create and initialize
MessageQueryTester* initialize()
{
  return new MessageQueryTester();
};

Anything else is unacceptable in my opinion.

If you see a lot of code like the one you posted in your code-base, I'm afraid it will require drastic measures to get rid of it, an exorcist maybe? (that's a joke, btw)


>>Now I could realize why there are so many languages would prefer to seal the power of the programmers.

When you have lethal weapons and an army of idiots, you have two choices if you don't want them to hurt each other. Either you get rid of the lethal weapons and loose your fire-power. Or, you get rid of the idiots by training them to use those weapons correctly. I prefer the latter, the designers of Java/C# have chosen the former.

Edited 4 Years Ago by mike_2000_17: n/a

Comments
great stuff :)
This article has been dead for over six months. Start a new discussion instead.