Sign in to follow this  
silverphyre673

Convert string to float (not quite typical! =) - solved

Recommended Posts

silverphyre673    454
Blah blah asked a million times. Anyways, I'm making a level loading system and need to load in some floating point numbers from a text file. A 3D vertex coordinate is stored like this in the file: [0 0 0] I've been using sprintf to convert it to 3 floating point errors, or trying to. The code looks like this, where "c" is a pointer to the character array of the vertex (it has the proper values - I checked): //c is created as a new character array on the heap, given the string loaded from //the file. sprintf(c, "[%f %f %f]", vertex.x, vertex.y, vertex.z); delete [] c; However, when I call delete on "c", the program segfaults and crashes. I checked the value of "c" in the debugger, and it changes like this: Loaded from file: [0 0 0] After sprintf: [0.000000 0.000000 0.000000] So when I call delete on it, the program crashes. I'm thinking that I would like a different method to convert "c" to a set of floating points in a similar manner to what I'm doing above. However, I would also like to avoid crashing the program. Any hints? Thanks! [Edited by - silverphyre673 on November 26, 2005 2:21:36 PM]

Share this post


Link to post
Share on other sites
Skeleton_V@T    512
Some possible reasons:
_ You mixed malloc () with delete
_ The allocated memory is not large enough for sprintf ()
_ Mistakes in calling new [] (I don't know for sure, please post your code)
_ The program stack segment is probably broken before sprintf () is executed, sprintf and c are just victims[smile].

sprintf () requires a char * pointer to be passed but it doesn't matter what is your real pointer type, just provide a cast for sprintf () and the compiler will silently compile your code.

Unless you intented to do something with c after used it to store floating point values, it's best to declare c as float * .

[Edit]
Are vertex.[x/y/z] 3 chars ?
What is the purpose of '[' and ']' in
sprintf(c, "[%f %f %f]", vertex.x, vertex.y, vertex.z);

Share this post


Link to post
Share on other sites
etothex    728
I don't see how you're loading and not saving - you have a char array containing vertices and you want to load it into vertex.[xyz], correct? Then you want to use sscanf, not sprintf. Or better yet, a stringstream. If you really do want to put the vertices into c, then continue reading below.

First - make sure that vertex.[xyz] are all floats and not some other data type. (unlikely, but with printf you just have to be sure)

Second - I would be leery of using sprintf. If you're using C++ (which you are - you have delete[]) then use a stringstream. Just create a new stringstream, do << "[ " << vertex.x << " " << vertex.y and so on. Then you can grab a string out of it using >> (some string) and doing .c_str() on the string. If you have a preexisting char array you can then do a strncpy to it.

If you must use sprintf, use snprintf instead (it's called something like _snprintf on msvc) That way if you are accidentally overwriting your buffer then you will get an incomplete string instead of a blown away stack.

Share this post


Link to post
Share on other sites
silverphyre673    454
I'm using C++, not C, and I might have gotten you, or you gotten me, confused between the variable "c" and the language =)

Anyways, I won't post my loading code, but suffice to say that the vertex coordinates are stored in the text file between brackets for readability. So if you were going to store a vertex, you would store it like this:

[0 15 3]

It is loaded into an std::string buffer, and I do this to store the string in a char *, so I can use it with sprintf:


struct Vertex
{
float x, y, z;
};

//...

//In loading function:

Vertex vertex;
std::string buffer = GetVertex(); //Load vertex value from text file, store
char * temp = new char[buffer.length() + 1]; //For sprintf
std::strcpy(temp, buffer.c_str()); //Copy string stored in "buffer" into "temp"
//temp now has the value "[0 15 3]"
sprintf(temp, "[%f %f %f]", vertex.x, vertex.y, vertex.z);
delete [] temp; //Program actually crashes on this line, but value of temp appears to change on the last line.




EDIT:

Ah, yes... I do want to put the string values into the float... Oops! Thanks!

Share this post


Link to post
Share on other sites
T2k    220

std::stringstream s << GetVertex();
s >> std::ws;
char p= s.peek();
if(p != '[')
std::error();
s.ignore();
s >> vertex.x >> vertex.y >> vertex.z;
s >> std::ws;
p= s.peek();
if(p != ']')
std::error();
//done


not tested, but you get the idea... boost has such stuff already in function form, but i dont use boost frequently enough to tell you the functionnames

and now: pls stop using c functions ;)

T2k

Share this post


Link to post
Share on other sites
Puzzler183    540
Actually, say what you want about type safety BUT:

1) C functions can be largely checked for type safety by the compiler.
2) C functions are significantly faster - not like twice as fast but orders of magnitude
3) C functions are simpler to use (if you know the formatting commands); why do you think they made boost::format? (which by the way, is still slow)

Share this post


Link to post
Share on other sites
me22    212
Quote:
Original post by Puzzler183
Actually, say what you want about type safety BUT:
1) C functions can be largely checked for type safety by the compiler.


void* and varargs, which C uses all over the place, are the anitithesis of type safety. In most cases they cannot be checked by the compiler, although in a few instances it's possible. Not to mention that printf has it's own vulnerability named after it. There's a reason you get "printf format string vulnerabilty" but never "iostream manipulater vulnerability".

Quote:
Original post by Puzzler183
2) C functions are significantly faster - not like twice as fast but orders of magnitude


That has nothing to do with them being "C" functions, it's just because they're either less safe, less general, or less extensible.

The String Formatters of Manor Farm
by Herb Sutter

Quote:
Original post by Puzzler183
3) C functions are simpler to use (if you know the formatting commands); why do you think they made boost::format? (which by the way, is still slow)


No, C functions have more succinct operations for formatting. However, they are still resticted to built-in types, so are not simpler to use with custom data types:
Vector3D v( 3, 4, 5 );
printf( "%?", v ); // uhoh. Dies no matter what you put for ?
cout << v; // works fine
cout << boost::format( "%+05s" ) % v; // also works fine


Also, c-style I/O can only be used for the limited cases that it allows for. In C++, you can have cout write zlib-compressed data with just:
zlib_ostreambuf<char> zsb( cout.rdbuf() );
cout.rdbuf( zsb );

And then using cout as you normally would.

Share this post


Link to post
Share on other sites
doynax    850
Quote:
Original post by me22
In most cases they cannot be checked by the compiler, although in a few instances it's possible. Not to mention that printf has it's own vulnerability named after it.
Really? GCC's format string validator works great, except perphaps for dynamic formats (but such things are very rare, and there's no equivalent functionality in standard C++ library).
Just don't forget to mark your functions as such when making a printf wrapper.

Share this post


Link to post
Share on other sites
iMalc    2466
Quote:
Original post by doynax
Quote:
Original post by me22
In most cases they cannot be checked by the compiler, although in a few instances it's possible. Not to mention that printf has it's own vulnerability named after it.
Really? GCC's format string validator works great, except perphaps for dynamic formats (but such things are very rare, and there's no equivalent functionality in standard C++ library).
Just don't forget to mark your functions as such when making a printf wrapper.
Checking the %? parts of the string passed to printf is only a weak compiler extension. Without having actually used it, I assume it wont work when your format string isn't a constant string, written directly into the function call, but is stored in some variable instead.

Share this post


Link to post
Share on other sites
doynax    850
Quote:
Original post by iMalc
Quote:
Original post by doynax
Quote:
Original post by me22
In most cases they cannot be checked by the compiler, although in a few instances it's possible. Not to mention that printf has it's own vulnerability named after it.
Really? GCC's format string validator works great, except perphaps for dynamic formats (but such things are very rare, and there's no equivalent functionality in standard C++ library).
Just don't forget to mark your functions as such when making a printf wrapper.
Checking the %? parts of the string passed to printf is only a weak compiler extension. Without having actually used it, I assume it wont work when your format string isn't a constant string, written directly into the function call, but is stored in some variable instead.
Yeah, that was what I meant by using a "dynamic format string". Still I can't see many cases where you'd use those (a positional equivalent would be nice for translations). I use printf all the time and I've never found a use for one.
And it's a compile-time check (unlike boost::format).

I'll admit that it is a hack but it works quit well practice. Hopefully more compilers will implement it in the future, so we can finally put an end to all of these complaints about printf's lack of type-safety.

Share this post


Link to post
Share on other sites
Will F    1069
Quote:
Original post by Puzzler183
2) C functions are significantly faster - not like twice as fast but orders of magnitude


For the sake of argument, in this example the level loading is only going to happen once per level. Unless I saw profiling data that indicated otherwise I would expect the difference here to be negligible.

Also since no one has mentioned it using boost::lexical_cast is another way to convert a string to a float.

Share this post


Link to post
Share on other sites
Puzzler183    540
If I really have to I'll dig up the site with the data on speed. They are pretty adequately checked and as for the printf vulnerability, that's actually for sprintf and if you're dumb enough to use sprintf instead of snprintf, well, suicide is the only option.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this