Click here to Skip to main content
15,917,481 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
See more:
Is this an OK way to do what it does?

C++
void CC::read(std::vector<SomeStructure> &info, Blob &data /*This is just some binary data imho*/)
{

 
   SomeStructure *ptr=(SomeStructure*) data.data();

 
    for(int i= 0;i<data.size()/sizeof(SomeStructure);i++)
    {
 
        info.push_back(*ptr);

 
        ptr++;
    }
}
Posted

The answer to your question lies in the definition of the Blob class. The code looks suspect but nothing in what you have presented here looks completely wrong. To complete the question, the details of Blob need to be provided.

If Blob actually consists of a contiguous array of SomeStructure, you are going about the problem wrong. There will likely be some alignment details you are not considering. Consecutive elements in Blob will likely have some padding that you are not considering. This detail is determined by the compiler and can't be reliably predicted.

In any situation where you try to outsmart the compiler, you will usually lose.
 
Share this answer
 
Comments
Dan page 3-Jul-13 10:31am    
Hi, the data inside the Blob class I think is stored as: void * data; (so it may indeed contain contigious array of SomeStructure objects).
This is not my code.
Sometimes they use this keyword in data structures: __packed (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CJAFJHJD.html) - maybe that is how they defeat issues with the padding?
thanks.
H.Brydon 3-Jul-13 10:45am    
Go for it. You might see differences between debug and release builds. Also different compiler versions.
Dan page 3-Jul-13 10:49am    
hi H.Brydon, what do you mean with "Go for it"? as I mentioned this is not my code, I am analyzing reading someone else's code; I might need to modify it at times though.
H.Brydon 3-Jul-13 11:14am    
Whoever wrote that should be shot. From a maintenance perspective, there are very minor changes that can be made to SomeStructure that will completely break this code. For example you can never add something to it that contains a pointer or method (or another class/struct containing a pointer or method). This code should not be used for persisting the struct/class. My guess is that it is for marshalling the data somewhere.
In my opinion this is an inefficient piece of code. As far as I understand it, it is supposed to do the following:

The Blob object contains binary data of known length which is a multiple of the length of SomeStucture. This entire chunk of data is copied into a vector with element type SomeStructure.

The code is inefficient for two reasons:

(a) The vector of SomeStructure is grown element by element. That requires multiple big copy operations that take place inside vector whenever the allocated space is exhausted.

(b) The operation push_back is executed with an element of SomeStructure as argument, which requires (depending on compiler optimization) multiple copy operations until the data arrives in the vector.

It would have been much more efficient to allocate the required size of the vector with a single operation and then to copy the entire blob with a single memcpy.

And all that under the assumption that the binary format of the Blob object is really compatible to an array of SomeStructure. If that code was in my project I would try to get rid of it.
 
Share this answer
 
Comments
Dan page 4-Jul-13 2:28am    
Hi, yes, but if we forget efficiency for a moment - it works at least right. It seemed bit suspicious to me too initially, but now that I look at it I think it will work. The cast will seem to work and ptr++ will also move to the next SomeStruct element appropriately.
nv3 4-Jul-13 3:19am    
Ok Dan, glad I could help.
Dan page 4-Jul-13 8:39am    
nv3 One more thing. You write:

"And all that under the assumption that the binary format of the Blob object is really compatible to an array of SomeStructure"

Now that I read it this is *really* important right, otherwise members of the SomeStructure will not get correctly initialized (if the format of Blob does not reflect structure of SomeStructure)?? right?? Thanks.
nv3 4-Jul-13 8:50am    
Yes, this is important. Otherwise your SomeStructure element will just contain garbage.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900