# string has wrong size

This topic is 2911 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Why doesn't this work?
typedef std::string str;

const str& SectorI::GetTileData() const
{
return
str("XXXXXXXXXXXXXXXX  XXXXXXXn") +
str("X      XXX    XX  XX    Xn") +
str("X      XXX              Xn") +
str("X      XXXXX          XXXn") +
str("X      XXX              Xn") +
str("X      XXX              Xn") +
str("X      XXXXXX   XX   XXXXn") +
str("X      XXX              Xn") +
str("X      XXXXXXXXXXXXXXXX Xn") +
str("X      XXXXXXXXXXXXXXXX Xn") +
str("X                       Xn") +
str("X                       Xn") +
str("X                       Xn") +
str("X                       Xn") +
str("X                       Xn") +
str("X                       Xn") +
str("X                       Xn") +
str("X                       Xn") +
str("XXXXXXXXXXXXXXXXXXXXXXXXXn");
}


The returned string has the expected data in it, however, it's size is incorrect, leading to all sorts of problems when I try to iterate over it. Specifically, GetTileData().size() returns 46419024 for some reason. Well actually, it's a different value each time, but it's always much bigger then it should be. It looks like an uninitialized variable, but that is internal to std::string and I think it is much more likely that I have a bug somewhere rather then there being a bug in the standard library. What am I doing wrong?

##### Share on other sites
You are returning a const reference!!!!!
WTF???????????????????????????????????????????????????????????????????????????????????????????????????????????????????????

##### Share on other sites
Oh I guess I should be returning by value in this case.
I got confused because temporaries are const references and I was returning a temporary variable. I forgot that you can't take references of a local variable and expect them to still be there.

##### Share on other sites
Quote:
 It looks like an uninitialized variable, but that is internal to std::string and I think it is much more likely that I have a bug somewhere rather then there being a bug in the standard library.

If you have a bug when you are using the standard library, then the bug most likely lies in your code, not in the std.

In this case, you are returning a reference on a local object which is allocated on the stack. So this object is destroyed after the return statement. Having the correct data in it is just pure luck.

##### Share on other sites
You can only return local variables by value.

And as a hint, string literals can be concatenated:

std::string SectorI::GetTileData() const{    return std::string(        "XXXXXXXXXXXXXXXX  XXXXXXXn"        "X      XXX    XX  XX    Xn"        "X      XXX              Xn"        "X      XXXXX          XXXn"//etc.        );}

##### Share on other sites
Quote:
Original post by Arkhyl
Quote:
 It looks like an uninitialized variable, but that is internal to std::string and I think it is much more likely that I have a bug somewhere rather then there being a bug in the standard library.

If you have a bug when you are using the standard library, then the bug most likely lies in your code, not in the std.

Yeah, that's what he said.

Besides, it would be better if he were to store tile data in a variable somewhere, rather that hard-code a function to return a copy of the data every time.
The SectorI class could have a member variable called tileData. That gets initialised in the constructor by calling GetTileData() then every other time it is needed, you only have to refer to the variable by reference. It also makes it easier in the future if you want to read tile data from a file.

##### Share on other sites
Incidentally, pretty much every compiler will issue a warning for this kind of thing. For instance, with MSVC you'd get a C4172, which is a level 1 warning. If you have warnings enabled at all, MSVC will complain that you're doing something wrong.

##### Share on other sites
Quote:
 Original post by XTAL256Besides, it would be better if he were to store tile data in a variable somewhere, rather that hard-code a function to return a copy of the data every time.
Incidentally this is one of those times where std::string just isn't needed, you could avoid the per-invocation construction/copying using a string literal:

const char * tile_data(){    return "XXXXXXXXXXXXXXXX  XXXXXXXn"           "X      XXX    XX  XX    Xn"           "X      XXX              Xn"           "X      XXXXX          XXXn"           "X      XXX              Xn"           "X      XXX              Xn"           "X      XXXXXX   XX   XXXXn"           "X      XXX              Xn"           "X      XXXXXXXXXXXXXXXX Xn"           "X      XXXXXXXXXXXXXXXX Xn"           "X                       Xn"           "X                       Xn"           "X                       Xn"           "X                       Xn"           "X                       Xn"           "X                       Xn"           "X                       Xn"           "X                       Xn"           "XXXXXXXXXXXXXXXXXXXXXXXXXn";}

##### Share on other sites
Quote:
Original post by dmatter
Quote:
 Original post by XTAL256Besides, it would be better if he were to store tile data in a variable somewhere, rather that hard-code a function to return a copy of the data every time.
Incidentally this is one of those times where std::string just isn't needed, you could avoid the per-invocation construction/copying using a string literal:

Depends on what you're going to do with that level data, but yeah. Then again, you'd store this sort of data in an external file. Data-driven development, no need to recompile if you just want to change some level-data.

##### Share on other sites
I don't remember seeing any compiler warning, although it may have been lost among the hundreds of spurious warnings that Box2d always generates.

I have thought about external storage for some of the level data but I have no idea how to do it. It doesn't help that there is a ton of level specific behavior.

##### Share on other sites
Quote:
 Original post by StoryyellerI don't remember seeing any compiler warning, although it may have been lost among the hundreds of spurious warnings that Box2d always generates.I have thought about external storage for some of the level data but I have no idea how to do it. It doesn't help that there is a ton of level specific behavior.

If you are using Visual Studio (i.e. MSVC compiler), you probably have to turn up the warning level to 1. You can do that in the settings somewhere.

If you are compiling Box2d from source with your project, then it should only need to compile once, then every other time it will just skip it as it is already compiled and hasn't been modified.

As for external storage, you can do that by reading from / writing to a simple text file using streams. But you probably don't have to worry about that just yet. Get your game working first, then work on improving it.

##### Share on other sites
Quote:
 Original post by XTAL256If you are compiling Box2d from source with your project, then it should only need to compile once, then every other time it will just skip it as it is already compiled and hasn't been modified.
Does Box2D actually generate that many warnings? I've never used it myself, but I'd hate to use a library which generated so many warnings as to make my compiler's output virtually useless... IMO library developers should be even more pedantic about compiler warnings than application developers.

##### Share on other sites
Quote:
 Original post by XTAL256If you are using Visual Studio (i.e. MSVC compiler), you probably have to turn up the warning level to 1. You can do that in the settings somewhere.
"up the warning level to 1"? I was horrified to discover that it is even possible to turn warnings off entirely. As if warning level 1 wasn't bad enough!
Projects are created with a warning level of 3 by default. Turning it down from that is nuts!

I certainly agree that library writers should take more care to ensure that their code compiles without warnings. If code using Box2D compiles with a number of warnings at level 1, and it's not something I was doing wrong, then I'd ditch it immediately.

##### Share on other sites
Quote:
 Original post by iMalc"up the warning level to 1"? I was horrified to discover that it is even possible to turn warnings off entirely. As if warning level 1 wasn't bad enough!Projects are created with a warning level of 3 by default. Turning it down from that is nuts!I certainly agree that library writers should take more care to ensure that their code compiles without warnings. If code using Box2D compiles with a number of warnings at level 1, and it's not something I was doing wrong, then I'd ditch it immediately.

Not sure where the OP is getting hundreds of warnings from. I get exactly zero errors or warnings compiling the latest version of Box2D at level 3.

##### Share on other sites
I remember a couple of minor warnings when compiling Box2D (latest stable) itself (just MS security and WP6 deprecated sort of things) but I've never had a warning arise from including the Box2D headers.

Storyyeller - you aren't recompiling Box2D into your project every build are you? It's far easier to build a static lib and link against that.

##### Share on other sites
I just compiled the latest version of Box 2D from scratch in Visual Studio with /W4, and got zero warnings from the library code.

Storyyeller, could you post some of the warnings that you're getting? It may be that you're doing something unusual that's causing the warnings to be generated.

##### Share on other sites
Quote:
Original post by iMalc
Quote:
 Original post by XTAL256If you are using Visual Studio (i.e. MSVC compiler), you probably have to turn up the warning level to 1. You can do that in the settings somewhere.
"up the warning level to 1"? I was horrified to discover that it is even possible to turn warnings off entirely. As if warning level 1 wasn't bad enough!
Projects are created with a warning level of 3 by default. Turning it down from that is nuts!

Wait, level 1 is the highest (i.e. shows all warnings), right? Or is it the lowest? Anyway, i meant for him to turn it to the setting which shows all warnings so that he could see the one SiCrane mentioned.

##### Share on other sites
Level 1 is the lowest level. If warnings are enabled at all MSVC will give a warning for this kind of thing.

##### Share on other sites
Actually, I'm using CodeBlocks, not MSVC

Here's a sample of the typical build log messages
..\zHeaders\BOX2D_21\Box2D\Collision\Shapes\b2CircleShape.h||In member function int32 b2CircleShape::GetSupport(const b2Vec2&) const':|
..\zHeaders\BOX2D_21\Box2D\Collision\Shapes\b2CircleShape.h||In member function const b2Vec2& b2CircleShape::GetSupportVertex(const b2Vec2&) const':|
..\zHeaders\BOX2D_21\Box2D\Collision\Shapes\b2CircleShape.h||In member function const b2Vec2& b2CircleShape::GetVertex(int32) const':|
..\zHeaders\BOX2D_21\Box2D\Dynamics\b2WorldCallbacks.h||In member function virtual void b2ContactListener::BeginContact(b2Contact*)':|
..\zHeaders\BOX2D_21\Box2D\Dynamics\b2WorldCallbacks.h||In member function virtual void b2ContactListener::EndContact(b2Contact*)':|
..\zHeaders\BOX2D_21\Box2D\Dynamics\b2WorldCallbacks.h||In member function virtual void b2ContactListener::PreSolve(b2Contact*, const b2Manifold*)':|
..\zHeaders\BOX2D_21\Box2D\Dynamics\b2WorldCallbacks.h||In member function virtual void b2ContactListener::PostSolve(b2Contact*, const b2ContactImpulse*)':|
..\zHeaders\BOX2D_21\Box2D\Dynamics\Joints\b2MouseJoint.h||In member function virtual bool b2MouseJoint::SolvePositionConstraints(float32)':|

##### Share on other sites
What compiler (and compiler version) are you using?

It looks to me as if all the warnings you're getting correspond to the B2_NOT_USED macro. Can you confirm this?

Presumably, the macro is intended to suppress 'unused variable' warnings. A little Googling indicates that the method used by the macro (a cast to void) will cause most compilers not to generate either warning (i.e. 'unused variable' or 'statement has no effect'), but maybe this doesn't work with whatever compiler/version combination you're using.

Your compiler may include an option to disable this particular warning. Or, you could redefine the macro in the source code so that it's 'empty', and then rebuild the library (although you may then get 'unused variable' warnings).

If you're concerned about it, you could post to the Box2D forums, and/or file an issue on the Google Code page. (Having done some open-source development myself, I can tell you that often times you only become aware of an issue when someone takes the time to report it.)

##### Share on other sites
It's actually a well known problem with Box2d when using GCC.

Someone suggested redefining the macro to the __unused attribute or something like that. Do you know how that works?

##### Share on other sites
There's some discussion of using the __unused attribute to fix the problem here.

I see that the issue has come up before, but do you know if an issue has ever been filed? I haven't checked myself, but you might look through the issue list to see if an issue was ever filed, and if so, what the outcome was. If no issue has been filed, I would recommend filing one (even though the author was apparently aware of the issue at one point, such things are easy to lose track of if they're not documented somewhere).

##### Share on other sites
Quote:
 Original post by StoryyellerOh I guess I should be returning by value in this case.I got confused because temporaries are const references

No, temporaries are values. You can pass them into a function that accepts a const reference, because of special rules. You can't return a reference, const or not, to a temporary, because a temporary will not outlive the function. You can't return a reference to a local variable, either, because it won't outlive the function either.

##### Share on other sites
However, as wrong as it looks...

int Foo(){	int temp = 0;	return temp;}int main(){	const int & ir = Foo();	std::cout << ir;}

...is completely valid. Const references to temporary return values forces the return value to not be destroyed right away. It's more-or-less move semantics. It can come in handy sometimes.

##### Share on other sites
May-be this helps to understand the latter scenario:
#include <cstdio>#include <cstdlib>struct X{    X() { std::puts("Default constructor"); }    X(const X&) { std::puts("Copy constructor"); }    ~X() { std::puts("Destructor"); }};X foo(){    std::puts("In foo");    if (std::rand() >= 0) { //this is a hack trying to suppress named return value optimization        X x;        std::puts("Returning from foo");        return x;    }    return X();}int main(){    {        std::puts("In main");        const X& x = foo();        std::puts("Leaving scope in main");    }    std::puts("Exiting");}

My output:

Quote:
 In mainIn fooDefault constructorReturning from fooCopy constructorDestructorLeaving scope in mainDestructorExiting

As you can see, when you leave foo, the local is destructed. But at the return, right before it goes out of scope, a copy is made. The reference binds to that copy and keeps the temporary alive until the const reference goes out of scope.