Putting data into a char* or std::string

your returning a stack allocated temporary array. That's undefined behavior, and your compiler is probably emitting a warning about that.

your returning a stack allocated temporary array. That's undefined behavior, and your compiler is probably emitting a warning about that.

You mean that I'm returning DataChar?
That's not really what's happening, I just wanted to explain it in a simple way... didn't think about it.

Edit:
Tested out the function I wrote now, it does give a warning.
Thanks for pointing it out, but this doesn't answer my question

There are several problems here: First, you cannot return a pointer to temporary memory -- the memory allocated for DataChar is invalidated as soon as Function returns, making it completely useless; second, while you can concatenate std::strings together, its more idiomatic in C++ to use stringsreams, and thirdly, with all of this combined together, you can use templates to make everything typesafe:


template<class Type>
istream& writeBinary( ostream& s, const Type& val ) {
s.write( (const char*)&val, sizeof(Type) );
return s;
}

template<>
istream& writeBinary( ostream& s, const string& str ) {
s.write( str.c_str(), str.length() );
s.put( '\0' );
return s;
}


stringstream ss;
writeBinary( ss, 5 );
writeBinary( ss, string( "This is a string" ) );
writeBinary( ss, 1.0f );
// etc..


EDIT:
I regrettably did not spend enough time fleshing out this post; Wahu's got my back though.

Well, kudos to you for trying. But it'll never work like that. floats and ints aren't stored as strings in memory, so you can't just convert them like that. Here are a couple popular options:

Using boost::lexical_cast (requires Boost)


int Integer = 532;
float Float = 3.141592f;
std::string buffer = "This is a string.";
buffer += boost::lexical_cast<std::string>(Integer);
buffer += boost::lexical_cast<std::string>(Float);


Using std::stringstream

float Float = 3.141592f;
std::stringstream stream;
stream << Float;
std::string buffer;
stream >> buffer;


[edit]

Oh goody, there were 3 posts in the time it took me to write 1! ninja'd += 3ish.

[edit edit]

Wait, are you wanting to convert it to strings or output it in its binary form?

There are several problems here: First, you cannot return a pointer to temporary memory -- the memory allocated for DataChar is invalidated as soon as Function returns, making it completely useless; second, while you can concatenate std::strings together, its more idiomatic in C++ to use stringsreams, and thirdly, with all of this combined together, you can use templates to make everything typesafe:


template<class Type>
istream& writeBinary( istream& s, const Type& val ) {
s.write( (const char*)&val, sizeof(Type) );
return s;
}

template<>
istream& writeBinary( istream& s, const string& str ) {
s.write( str.c_str(), str.length() );
s.put( '\0' );
return s;
}


stringstream ss;
writeBinary( ss, 5 );
writeBinary( ss, string( "This is a string" ) );
writeBinary( ss, 1.0f );
// etc..


and it breaks silently the first time you pass in a char* by accident. also, use reinterpret_cast, just to be more C++y

There are several problems here: First, you cannot return a pointer to temporary memory -- the memory allocated for DataChar is invalidated as soon as Function returns, making it completely useless; second, while you can concatenate std::strings together, its more idiomatic in C++ to use stringsreams, and thirdly, with all of this combined together, you can use templates to make everything typesafe:


template<class Type>
istream& writeBinary( istream& s, const Type& val ) {
s.write( (const char*)&val, sizeof(Type) );
return s;
}

template<>
istream& writeBinary( istream& s, const string& str ) {
s.write( str.c_str(), str.length() );
s.put( '\0' );
return s;
}


stringstream ss;
writeBinary( ss, 5 );
writeBinary( ss, string( "This is a string" ) );
writeBinary( ss, 1.0f );
// etc..


and it breaks silently the first time you pass in a char* by accident. also, use reinterpret_cast, just to be more C++y

I see. Thanks, I'll experiement with that.
And Washu, what do you mean?
I have never used reinterpret_cast before... I can look it up I guess ^^

Ok, I will simply assume that you do NOT want to convert your data into an actual string and have int x = 20 turn into char* = "20".

So first, your function is rather pointless and doesn't do anything that brute force casting your data to char* wouldn't do (without blowing up the stack with a ridiculously large array and pointless copying).

Second, even if this WOULD be working, you at no point seem to terminate that pseudo string you create with a 0. Trying to concatenate it with a string will just keep reading and copying until it hits the first zero that happens to be in memory. Best case: access violation to let you know there is a problem, worst case: seems to work most of the time and in debug builds, but suddenly sometimes breaks or copies garbage.

Third: please don't abuse the string class like that. If you want nothing but a generic buffer for data, use vector. Anybody seeing "string" expects it to contain actual text, not raw binary data (also, most of the functions of string don't make any sense or will just break in a thousand ways).

The ugly way to do it quick and dirty would be something like:

buffer.insert( buffer.end(), (char*)&MyData, ((char*)&MyData) + sizeof(MyData) );

Now this contains lots of ugly code, should also use a reinterpret_cast(&MyData), probably be split over 2 lines and it will still break if you use it on anything but native types (ie. do NOT try to use it on c-strings or arrays or stl containers). Yes, it could be used for structs, but if you try to use it to pass data to another program it can break (since you can't rely on it having the same layout in memory). Pragma pack should be looked at. Also byte order if using different platforms. It will also fail hard if your struct contains pointers or anything that requires more than a shallow copy.

While this might look a little cleaner, I would still consider it a case of "please only do this if you really know what you're doing":
const char* rawPtr = reinterpret_cast(&MyData);
buffer.insert( buffer.end(), rawPtr, rawPtr + sizeof(MyData) );


You can use templates to hide it and save some work, but should be very careful to have specializations for some types (strings, pointers, arrays) and think REALLY hard about what happens if someone uses this on classes with virtual functions, containing stl containers, etc.

The real question might be: what exactly are you trying to do and what do you need that buffer for?

Here is an example that solves some of the problems with fastcall's code. Mainly it deals with the issue of char*, and also with pointer type problems (by using SFINAE). It also fixes the erroneous use of istream instead of ostream.

#include <iostream>
#include <string>
#include <type_traits>
#include <sstream>
#include <cstdlib>
template<class T>
typename std::enable_if<std::is_fundamental<T>::value, std::ostream>::type&
write_binary(std::ostream& s, T const& value) {
s.write(reinterpret_cast<char const*>(&value), sizeof(T));
return s;
}
template<class CharType, class CharTraits>
std::ostream&
write_binary(std::ostream& s, std::basic_string<CharType, CharTraits> const& str) {
s.write(reinterpret_cast<char const*>(&str.front()), str.length() * sizeof(CharType));
return s;
}
std::ostream& write_binary(std::ostream& s, std::string const& str) {
s.write(reinterpret_cast<char const*>(&str.front()), str.length());
return s;
}
std::ostream& write_binary(std::ostream& s, std::wstring const& str) {
s.write(reinterpret_cast<char const*>(&str.front()), str.length() * sizeof(std::wstring::value_type));
return s;
}
std::ostream& write_binary(std::ostream& s, char const* data, size_t length) {
s.write(data, length);
return s;
}
struct S {
int a;
char* b;
};
int main() {
std::ostringstream ss;
write_binary(ss, 5);
int mynumber = rand();
write_binary(ss, mynumber);
write_binary(ss, "Hello world");
write_binary(ss, L"Hello world");
write_binary(ss, "Hello world", 11);
// write_binary(ss, &mynumber); // fails, pointer to T is excluded by enable_if, thus no appropriate overload is found.
// write_binary(ss, S); // fails, S is not a fundamental type. if an overload was available it would succeed though.
for(auto val : ss.str()) {
std::cout<<std::hex<<static_cast<int>(val)<<" ";
}
std::endl(std::cout);
}




int WriteBinary(FILE* fp,uint64_t value)
{
return fwrite(&value,1,8,fp);
}

int WriteBinary(FILE* fp,const char* value)
{
return fwrite(value,1,strlen(value),fp);
}

int WriteBinary(FILE* fp,char* value)
{
return WriteBinary(fp,(const char*)value);
}

int WriteBinary(FILE* fp,void* d,uint32_t l)
{
return fwrite(d,1,l,fp);
}

int WriteBinary(FILE* fp,const string& value)
{
return WriteBinary(fp,(const char*)value.c_str());
}

int main()
{
FILE* fp = fopen("file","wb");
double d = 1234.56789;
float f = 1234.1234f;
int i = 1234;
long long int l = 99999999;
const char* s = "gggggg";
string ss = "ccccc";
const char* sss = "????";

WriteBinary(fp,d);
WriteBinary(fp,f);
WriteBinary(fp,i);
WriteBinary(fp,l);
WriteBinary(fp,s);
WriteBinary(fp,ss);
WriteBinary(fp,sss,strlen(sss));
fclose(fp);
return 0;
}

What do i win?

Edit1 : if you would like to store strings first write their length followed by the literal.

Thanks for all the replies
I'll try to write a function using the information in your posts.

@Trienco:
Your assumption is correct. Thanks for the information, and I'm using it to send WinSock packets.

@Washu:
Thanks, I'll look it over.

@vNeeki:
Thanks, that looks clean to me.


By the way, there are some functions I haven't read about, so I'll do that now.
I'll reply back later with my progress
Thanks again for the replies ^^

Edit:
It works by the way, thanks to all of you for help ^^

[quote name='fastcall22' timestamp='1329614141' post='4914383']
There are several problems here: First, you cannot return a pointer to temporary memory -- the memory allocated for DataChar is invalidated as soon as Function returns, making it completely useless; second, while you can concatenate std::strings together, its more idiomatic in C++ to use stringsreams, and thirdly, with all of this combined together, you can use templates to make everything typesafe:


template<class Type>
istream& writeBinary( istream& s, const Type& val ) {
s.write( (const char*)&val, sizeof(Type) );
return s;
}

template<>
istream& writeBinary( istream& s, const string& str ) {
s.write( str.c_str(), str.length() );
s.put( '\0' );
return s;
}


stringstream ss;
writeBinary( ss, 5 );
writeBinary( ss, string( "This is a string" ) );
writeBinary( ss, 1.0f );
// etc..


and it breaks silently the first time you pass in a char* by accident. also, use reinterpret_cast, just to be more C++y
[/quote]

That depends on your definition of break. I personally think it did what I'd expect it to do, which is write the address to the stream. A write method, particularly one named binary write, shouldn't really try to infer what you mean when you pass in a char*. If you pass in an int* and it writes the address to the stream, that is expected behavior - why should char* be any different (certainly for binary streams)?

Here is an example that solves some of the problems with fastcall's code. Mainly it deals with the issue of char*, and also with pointer type problems (by using SFINAE). It also fixes the erroneous use of istream instead of ostream.

#include <iostream>
#include <string>
#include <type_traits>
#include <sstream>
#include <cstdlib>
template<class T>
typename std::enable_if<std::is_fundamental<T>::value, std::ostream>::type&
write_binary(std::ostream& s, T const& value) {
s.write(reinterpret_cast<char const*>(&value), sizeof(T));
return s;
}
template<class CharType, class CharTraits>
std::ostream&
write_binary(std::ostream& s, std::basic_string<CharType, CharTraits> const& str) {
s.write(reinterpret_cast<char const*>(&str.front()), str.length() * sizeof(CharType));
return s;
}
std::ostream& write_binary(std::ostream& s, std::string const& str) {
s.write(reinterpret_cast<char const*>(&str.front()), str.length());
return s;
}
std::ostream& write_binary(std::ostream& s, std::wstring const& str) {
s.write(reinterpret_cast<char const*>(&str.front()), str.length() * sizeof(std::wstring::value_type));
return s;
}
std::ostream& write_binary(std::ostream& s, char const* data, size_t length) {
s.write(data, length);
return s;
}
struct S {
int a;
char* b;
};
int main() {
std::ostringstream ss;
write_binary(ss, 5);
int mynumber = rand();
write_binary(ss, mynumber);
write_binary(ss, "Hello world");
write_binary(ss, L"Hello world");
write_binary(ss, "Hello world", 11);
// write_binary(ss, &mynumber); // fails, pointer to T is excluded by enable_if, thus no appropriate overload is found.
// write_binary(ss, S); // fails, S is not a fundamental type. if an overload was available it would succeed though.
for(auto val : ss.str()) {
std::cout<<std::hex<<static_cast<int>(val)<<" ";
}
std::endl(std::cout);
}


What happens when I pass in an unterminated char* string, say the contents of a vertex buffer? Now you have a buffer overrun. What if I want to write an int* array, just like I write a char* array?

To guard against user error in fastcalls code, I'd be more inclined to add the following


template<class Type>
istream& writeBinary( istream& s, const Type* val);


Now you cannot accidentally call passing in a pointer without size, instead you'll get a link error. You can optionally define the following block to handle array situations


template<class Type>
istream& writeBinary( istream& s, const Type* val, int num_elements)
{
// do something
return s;
}
template<class Type>
istream& writeBinaryAsText( istream& s, const Type* val)
{
// treat input array as null terminated string..
}




That depends on your definition of break. I personally think it did what I'd expect it to do, which is write the address to the stream. A write method, particularly one named binary write, shouldn't really try to infer what you mean when you pass in a char*. If you pass in an int* and it writes the address to the stream, that is expected behavior - why should char* be any different (certainly for binary streams)?

It's a bug almost 100% of the time. Writing out the pointer is never the right idea. In fact, its such a bug that many modern compilers have tests for code like that and display a nice large warning on it.

What happens when I pass in an unterminated char* string, say the contents of a vertex buffer? Now you have a buffer overrun. What if I want to write an int* array, just like I write a char* array?[/quote]
Then you should pay more attention to where you're throwing random char*'s around. OR, better yet, you should be using a std::vector since raw pointers are a silly idea anyways. If you want to write an integer array that's "null terminated" then you can, just write the appropriate overload.

To guard against user error in fastcalls code, I'd be more inclined to add the following


template<class Type>
istream& writeBinary( istream& s, const Type* val);


Now you cannot accidentally call passing in a pointer without size, instead you'll get a link error. You can optionally define the following block to handle array situations
[/quote]
Indeed, instead you have a confusing linker error that provides no diagnostics as to where the heck you went wrong. If you're going to emit an error, do it right:


template<class T>
std::ostream& write_binary(std::ostream& s, T* ptr) {
static_assert(false, "Error: expected overload not found.");
}

Now you have a nice compile time error that provides a backtrace to the exact line where you went wrong:


1>program.cpp(17): error C2338: Error: expected overload not found.
1> program.cpp(47) : see reference to function template instantiation 'std::ostream &write_binary<const char>(std::ostream &,T *)' being compiled
1> with
1> [
1> T=const char
1> ]
1>program.cpp(17): error C2338: Error: expected overload not found.
1> program.cpp(48) : see reference to function template instantiation 'std::ostream &write_binary<const wchar_t>(std::ostream &,T *)' being compiled
1> with
1> [
1> T=const wchar_t
1> ]

and while we're at it, we might as well add some iterator versions as well:
template<class Itor>
typename std::enable_if<
!std::is_pointer<Itor>::value &&
std::is_fundamental<typename std::iterator_traits<Itor>::value_type>::value
, std::ostream>::type&
write_binary(std::ostream& s, Itor begin, Itor end) {
if(begin == end)
return s;

if(std::is_same<typename std::iterator_traits<Itor>::iterator_category, std::random_access_iterator_tag>::value) {
s.write(reinterpret_cast<char const*>(&*begin), std::distance(begin, end) * sizeof(*begin));
} else {
for(; begin != end; ++begin) {
s.write(reinterpret_cast<char const*>(&*begin), sizeof(*begin));
}
}
return s;
}

template<class Itor>
typename std::enable_if<
std::is_pointer<Itor>::value &&
std::is_fundamental<typename std::iterator_traits<Itor>::value_type>::value
, std::ostream>::type&
write_binary(std::ostream& s, Itor begin, Itor end) {
s.write(reinterpret_cast<char const*>(begin), (end - begin) * sizeof(*begin));
return s;
}

Now you can handle containers, like vector and list. Requires the types to be fundamental, thus you must provide appropriate overloads for custom types.

It's a bug almost 100% of the time. Writing out the pointer is never the right idea. In fact, its such a bug that many modern compilers have tests for code like that and display a nice large warning on it.

Sure it's likely not the behavior the user had in mind, but the client is doing the wrong thing, the program shouldn't function correctly. I 100% agree the interface should be adapted to prevent this kind of user error, what I disagree with is adapting it to accept incorrect inputs gracefully. By doing this, users might well erronously assume that other methods will magically treat const char* buffers as null terminated strings

What happens when I pass in an unterminated char* string, say the contents of a vertex buffer? Now you have a buffer overrun. What if I want to write an int* array, just like I write a char* array?

Then you should pay more attention to where you're throwing random char*'s around. OR, better yet, you should be using a std::vector since raw pointers are a silly idea anyways. If you want to write an integer array that's "null terminated" then you can, just write the appropriate overload.
[/quote]

Locking vertex buffers typically has the result dumped into void*, and treating this as unsigned char* is (rightly or wrongly) quite common practice. It's not a huge leap to see someone trying to pass into into write_binary. Yes it's user error - but a null terminated string is a text concept, not a binary stream thing, I just believe they should be kept distinct.

Indeed, instead you have a confusing linker error that provides no diagnostics as to where the heck you went wrong. If you're going to emit an error, do it right:


template<class T>
std::ostream& write_binary(std::ostream& s, T* ptr) {
static_assert(false, "Error: expected overload not found.");
}

Now you have a nice compile time error that provides a backtrace to the exact line where you went wrong:

Agreed this is a big improvement.

EDIT: one final thought is how would you handle read_binary(std::istream& s, char* value)? There is no sensible implementation, as a result it would break symmetry (consistency) of the read/write interface.

EDIT: one final thought is how would you handle read_binary(std::istream& s, char* value)? There is no sensible implementation, as a result it would break symmetry (consistency) of the read/write interface.

well, first off, if we're expected to read this data, we need to change the data we write in a few ways.
Firstly, strings need to be either null terminated (bad idea) or length prefixed (good idea). Secondly, no, you wouldn't return data in that manner. You would instead identify the interface you wish to provide (such as taking a std::vector& and a length) and then implement that. Such as:


#include <iostream>
#include <string>
#include <type_traits>
#include <sstream>
#include <cstdlib>
#include <vector>
#include <iterator>

template<class T>
typename std::enable_if<std::is_fundamental<T>::value, std::ostream>::type&
write_binary(std::ostream& s, T const& value) {
s.write(reinterpret_cast<char const*>(&value), sizeof(T));
return s;
}

template<class T>
std::ostream& write_binary(std::ostream& s, T* ptr) {
static_assert(false, "Error: expected overload not found.");
}

template<class CharType, class CharTraits>
std::ostream& write_binary(std::ostream& s, std::basic_string<CharType, CharTraits> const& str) {
std::basic_string<CharType, CharTraits>::size_type size = str.length();
s.write(reinterpret_cast<char const*>(&size), sizeof(std::basic_string<CharType, CharTraits>::size_type));
s.write(reinterpret_cast<char const*>(&str.front()), str.length() * sizeof(CharType));
return s;
}

std::ostream& write_binary(std::ostream& s, std::string const& str) {
std::string::size_type size = str.length();
s.write(reinterpret_cast<char const*>(&size), sizeof(std::string::size_type));
s.write(reinterpret_cast<char const*>(&str.front()), str.length());
return s;
}

std::ostream& write_binary(std::ostream& s, std::wstring const& str) {
std::wstring::size_type size = str.length();
s.write(reinterpret_cast<char const*>(&size), sizeof(std::wstring::size_type));
s.write(reinterpret_cast<char const*>(&str.front()), str.length() * sizeof(std::wstring::value_type));
return s;
}

std::ostream& write_binary(std::ostream& s, char const* data, size_t length) {
s.write(data, length);
return s;
}

template<class Itor>
typename std::enable_if<
std::is_fundamental<typename std::iterator_traits<Itor>::value_type>::value
, std::ostream>::type&
write_binary(std::ostream& s, Itor begin, Itor end) {
if(begin == end)
return s;

if(std::is_same<typename std::iterator_traits<Itor>::iterator_category, std::random_access_iterator_tag>::value) {
s.write(reinterpret_cast<char const*>(&*begin), std::distance(begin, end) * sizeof(*begin));
} else {
for(; begin != end; ++begin) {
s.write(reinterpret_cast<char const*>(&*begin), sizeof(*begin));
}
}
return s;
}

template<class T>
typename std::enable_if<std::is_fundamental<T>::value, std::istream>::type&
read_binary(std::istream& s, T& outvalue) {
s.read(reinterpret_cast<char*>(&outvalue), sizeof(T));
return s;
}

template<class T>
std::istream& read_binary(std::istream& s, T* ptr) {
static_assert(false, "Error: expected overload not found.");
}

template<class CharType, class CharTraits>
std::istream& read_binary(std::istream& s, std::basic_string<CharType, CharTraits>& str) {
std::basic_string<CharType, CharTraits>::size_type size;
s.read(reinterpret_cast<char const*>(&size), sizeof(std::basic_string<CharType, CharTraits>::size_type));
str.resize(size);
s.read(reinterpret_cast<char const*>(&str.front()), str.length() * sizeof(CharType));
return s;
}

std::istream& read_binary(std::istream& s, std::string& str) {
std::string::size_type size;
s.read(reinterpret_cast<char*>(&size), sizeof(std::string::size_type));
str.resize(size);
s.read(reinterpret_cast<char*>(&str.front()), str.length());
return s;
}

std::istream& read_binary(std::istream& s, std::wstring& str) {
std::wstring::size_type size;
s.read(reinterpret_cast<char*>(&size), sizeof(std::wstring::size_type));
str.resize(size);
s.read(reinterpret_cast<char*>(&str.front()), str.length() * sizeof(std::wstring::value_type));
return s;
}

std::istream& read_binary(std::istream& s, std::vector<char>& data, size_t length) {
data.resize(length);
s.read(reinterpret_cast<char*>(&data.front()), data.size());
return s;
}

template<class Itor>
typename std::enable_if<std::is_fundamental<typename std::iterator_traits<Itor>::value_type>::value, std::istream>::type&
read_binary(std::istream& s, Itor begin, Itor end) {
if(begin == end)
return s;

if(std::is_same<typename std::iterator_traits<Itor>::iterator_category, std::random_access_iterator_tag>::value) {
s.read(reinterpret_cast<char*>(&*begin), std::distance(begin, end) * sizeof(*begin));
} else {
for(; begin != end; ++begin) {
s.read(reinterpret_cast<char*>(&*begin), sizeof(*begin));
}
}
return s;
}

int main() {
std::ostringstream oss;
write_binary(oss, 5);
int mynumber = rand();
write_binary(oss, mynumber);
write_binary(oss, "Hello world", 11);
write_binary(oss, std::string("Hello world"));

// write_binary(oss, "Hello world"); // error, no appropriate overload found.
#ifdef _MSC_VER
for each(auto val in oss.str()) {
std::cout<<std::hex<<static_cast<int>(val)<<" ";
}
#else
for(auto val : oss.str()) {
std::cout<<std::hex<<static_cast<int>(static_cast<unsigned char>(val))<<" ";
}
#endif
std::endl(std::cout);

std::istringstream iss(oss.str());
int value;
read_binary(iss, value);
std::cout<<"Should be 5: "<<value<<std::endl;
read_binary(iss, value);
std::cout<<"Should be random: "<<value<<std::endl;
std::vector<char> data(11);
read_binary(iss, data.begin(), data.end());
// Alternatively:
// std::vector<char> data;
// read_binary(iss, data, 11);
std::cout<<"Should be 'Hello world': "<<std::string(data.begin(), data.end())<<std::endl;
std::string str;
read_binary(iss, str);
std::cout<<"Should be 'Hello world': "<<str<<std::endl;

// char* ptr;
// read_binary(iss, ptr); // error, no appropriate overload found.
}


I want to put ints, doubles etc. into a char array, for a buffer.

I've got a feeling i missunderstood your question or your intentions. But no one in here has yet suggested snprintf, which seems to be the most logical and simple answer.