using vector to make a string mutable

Started by
3 comments, last by yewbie 14 years, 11 months ago
OK stick with me here I am in way over my head and don't really understand what I am doing wrong... basically im trying to convert my winsock client and server over to C++ using strings etc since its a lot safer than using char for everything. This is just a test program I made to try to see if I could get the data manipulation working like I did with chars.. Basically what I did was create a class that gets called with 3 variables, Size, ID, and data. I want the constructor to take those and add them to the string in that order like so WORD Size -> WORD ID -> rest of data but it doesn't seem to work how I have it, first if I don't initialize the string with some garbage data I cannot just modify the first 4 bytes of data, using a suggestion from someone here on the forums I tried using vector to reinterpret_cast a pointer to modify the data but I think im missing something, any help would be greatly appreciated (I think its something fundamental im not understanding about how this should work)

// DataTest.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "windows.h"
#include <stdio.h>
#include <io.h>
#include <FCNTL.H>
#include <iostream>
#include <fstream>
#include <string>
#include <list>
#include <vector>
using namespace std;

//-----------------------------------------------------------------------------------//
//this class holds each of our packets that we are sending to the server
class OutgoingPacket
{
	private:
	public:
		string	PacketData;			//this holds all of our packet information
			
	OutgoingPacket(WORD MySize, WORD MyPacketID, string Mystring)//Constructor
		{
			PacketData.reserve(2048);
			PacketData=Mystring;
			std::vector<char> v(PacketData.begin(),PacketData.end());  //turn it into mutable
			WORD *Mysizetemp=reinterpret_cast<WORD*>(&(v[0])); //grab pointer to our Size
			WORD *MyPacketIDtemp=reinterpret_cast<WORD*>(&(v[2]));//grab pointer to our Packet ID
			
			*Mysizetemp = MySize;
			*MyPacketIDtemp= MyPacketID;
			
			//PacketData = (Mystring.substr(2,Mystring.size())); //send all the packet data to the rest the buffer			
	
		}
	~OutgoingPacket()	//Destructor
		{
			
		}
};



int _tmain(int argc, _TCHAR* argv[])
{
	OutgoingPacket op(8,1,"test");
	
	WORD PacketSize =(WORD)(op.PacketData[0]);
	WORD PacketID = (WORD) (op.PacketData[2]);
	//string newstring = opo.PacketData.substr(2,opo.PacketData.size());

	cout << "PacketSize = " << PacketSize << endl;
	cout << "PacketID = " << PacketID << endl;
	//cout << "Data = " << newstring << endl;
	
	return 0;
}


Advertisement
Well you never said exactly what was wrong, but your problem is that this is what you did:

copy MyString to the vector
overwrite the vector
print the PacketData

You forgot a step, I'll let you figure out where to put it:
copy the vector to PacketData

edited for clarity

C++: A Dialog | C++0x Features: Part1 (lambdas, auto, static_assert) , Part 2 (rvalue references) , Part 3 (decltype) | Write Games | Fix Your Timestep!

I replied to your latest PM with some suggestions on the initialisation of the buffer. For the sake of the thread, this is the rough bodge code I suggested to make constructing the buffer a bit easier:

#include <vector>#include <string>#include <cstring>#include <iostream>#include <windows.h> // for WORD typedefclass buffer{private:    void write(const char *v,size_t sz);    std::vector<char> data;    size_t p;public:    buffer() : p(0) { }    template<class T> buffer &operator<<(const T &t){ write(reinterpret_cast<const char*>(&t),sizeof(T)); return *this; } // for POD types    buffer &operator<<(const char *s){ write(s,std::strlen(s)); return *this; } // char arrays and strings handled a bit differently to PODs    buffer &operator<<(const std::string &s){ write(s.c_str(),s.size()); return *this; }    std::string str() const { return std::string(data.begin(),data.begin()+p); }};void buffer::write(const char *v,size_t sz){    if(p+sz>data.size()) data.resize(data.size()+sz);    char *s=reinterpret_cast<char*>(&(data));    std::copy(v,v+sz,s);    p+=sz;}int main(){    buffer b;        b << WORD(100) << WORD(200) << "abcdefg";        std::string s=b.str();        for(size_t i=0;i<s.size();++i) std::cout << int(s) << "\n";}


However, there is also a problem with the way you are reading the values back in your main() function:

WORD PacketSize=(WORD)(op.PacketData[0]);WORD PacketID=(WORD)(op.PacketData[2]);


Consider - op.PacketData is a std::string, so the type returned by std::string::operator[] is a char. So rather than taking the two raw bytes starting from each index and casting them to a WORD, you are taking a single char and statically converting it to a word.

You need to instead do some nasty casting:

WORD PacketSize=*(reinterpret_cast<WORD*>(&(op.PacketData[0])));


to get what you want.

Working from the inside out, take the address of the relevant byte in PacketData, cast the address to pointer-to-WORD then use indirection operator to get the value pointed at.

(It's all a bit horrid, but for those reading this that aren't aware, yewbie is trying to encode and decode binary information into character buffers for networking purposes.)

The reason I suggested to you in my PM that you post rather than PM about this is that I'm sure plenty of other people have a lot more experience of this than me and can give constructive advice.

BTW, I'm not really a networking expert but would it not be easier to just send ASCII text strings that you encode/decode using std::stringstreams rather than raw binary information with all its associated gotchas? Or does the increase in string size make this prohibitive?
You have two technical problems with the creation of the packet:

1) You overwrite the beginning of the text with the size and ID information. What you want to do is write the size and ID into the vector, and then append the text to the end of that.

2) You save the input string to the member string, create a vector from the member string and then modify the vector. This does not affect the member string because the vector contains a *copy* of the character data. What you want to do is write everything into the vector first, and *then* assign the *contents of the vector* to the member string. This can be done with the .assign() member function of std::string.

You have one technical problem with the decoding of the packet:

1) What Aardvajk said.

You have one fundamental design problem:

1) There is no point in doing it like this. std::string instances are mutable. The reason we copy std::strings into std::vectors is to make a mutable copy of the data in char-buffer form in order to pass to APIs that expect a non-const char*. You don't have this requirement. Yet. Where you will likely have the requirement is at the point where you actually send the packet over the network.

Have the OutgoingPacket just save the ID and the text data. Don't do anything fancy with that yet.

When it's time to send (I assume you will eventually have a member function called .send(), or something like that), do the following:

1) Construct an empty vector.
2) Write 2 bytes to the beginning of the vector representing the size of the text. You can use .push_back() for this, along with a little arithmetic to calculate each byte of the WORD.
3) Write the ID after that, in similar fashion.
4) Use std::copy() to put the actual string data at the end of the vector. Use a std::back_insert_iterator for the destination (use the free function std::back_inserter() to create it); that way, the vector will automatically resize itself as the string data is copied in.
5) Pass a pointer to the beginning of the vector's internal buffer to the API call.

The whole thing looks like this:

void appendWordTo(std::vector<char>& buffer, WORD word) {  // Left as an exercise.}class Packet {  std::string text;  WORD id;  // Notice how strings are passed by const reference. Also notice the  // use of the initialization list.  Packet(const std::string& text, WORD id) : text(text), id(id) {}  // Here "Destination" and "APISend" are placeholders for whatever it is that  // you actually have to call. Use your head. :)  void Send(const Destination& d) {    std::vector buffer;    // I assume "size" is the total packet size here, including the size    // field itself and the ID. Your conventions may differ.    WORD size = buffer.size() + 4;    appendWordTo(buffer, size);    appendWordTo(buffer, id);    // This is the fun part.    std::copy(text.begin(), text.end(), std::back_inserter(buffer));    APISend(d, &buffer.front());  }};// And here's a free bonus for you.Packet Receive(const Source& s) {  WORD size = APIReadWordFrom(s);  WORD id = APIReadWordFrom(s);  // Allocate space for the text data.  std::vector<char> buffer(size - 4);  APIReadBufferFrom(s, &buffer.front());  // After reading the data, we use it to construct a string, and use the  // string to construct a Packet representing what we received.  return Packet(std::string(buffer.begin(), buffer.end()), id);}
Thanks for the reply guys, yeah I think the way I was going about it was all wrong, I am gonna read up on on vectors.

That looks good, there is so much change from using char's that it almost seems hopeless =p

This topic is closed to new replies.

Advertisement