Having a problem with a vector iterator

Started by
18 comments, last by Zahlman 15 years, 5 months ago
Quote:Original post by dphoenix
This is the function to get myIP:

const char* Messenger::getMyIP(){    hostent* localInfo = gethostbyname("");  // get the local host name    struct in_addr addr;    memcpy(&addr,localInfo->h_addr_list[0],sizeof(struct in_addr));    const char* localIP = inet_ntoa(addr);    return localIP;}


I've confirmed that that part works.
What if you call that function 2 or 3 times? I bet it returns the same pointer each time, meaning if you store the pointers in a vector, you'll end up with all entries in the vector being pointers to the same character, meaning they'll seem to be the same string.

Quote:Original post by dphoenix
Also, whenever another player sends me a message, the message automatically has their IP address attached by the TCP listener that gets the message:

string mymessage(buffer);size_t found = mymessage.find("#");if((found != string::npos) && (found > 0))//found # delimiter{                Message m;                int pos = int(found);                string codeSub = mymessage.substr(0, pos);                m.code = codeSub;                m.message = mymessage.substr(pos+1);                m.sender = host;                messageList.push_back(m);                //cout<<"Message in queue."<<endl;}


The preceding code is in the TCP listener, when it gets a message from another player. Each message has a code and the content, all delineated by the # sign. So that's what m.code and m.message are. Each message also has a space for the sender address, which is initially uninitialized when its sent (this was before I knew how to get my own IP address). m.sender is also a const char*,
If m.sender is a const char*, it's just storing the pointer, not the contents. What is host?
Also, the test for fount > 0 and the cast of found to an it is redundant (std::string::find will always return npos or >= 0, and substr() takes size_t parameters rather than ints I think (Don't have VC to check though)).

Quote:Original post by dphoenix
class Message{    public:        Message();        Message(string c, string m);    public:        string code;        string message;        const char* sender;};Message::Message(){}Message::Message(string c, string m):code(c),message(m){}


That's what it looks like. I hope I didn't post too much irrelevant code, but I want to make sure I'm providing enough info because I am absolutely at my wit's end and ready to cry right now lol...
[smile]

Basically, whenever you store a const char* you're storing the pointer, not the contents. If you're re-using a buffer (Like the sprintf() example in my last post), then you're just storing the address of the original buffer, not a copy of it.
Advertisement
I just tried something.

I tried both of the following statements:

cout<<(battleList.front() == myIP)<<endl;cout<<(battleList.back() == myIP)<<endl;


Front returned false, back returned true. So it is the data being changed and not the pointers themselves...
Quote:Original post by dphoenix
This is the function to get myIP:

const char* Messenger::getMyIP(){    hostent* localInfo = gethostbyname("");  // get the local host name    struct in_addr addr;    memcpy(&addr,localInfo->h_addr_list[0],sizeof(struct in_addr));    const char* localIP = inet_ntoa(addr);    return localIP;}


I've confirmed that that part works.


Here's the thing: it doesn't.

First of all, the function is deprecated, as noted there. But more importantly,

Quote:inet_ntoa() returns the dots-and-numbers string in a static buffer that is overwritten with each call to the function.


(boldface original, italic emphasis mine.)

Messenger::getMyIP() actually always returns the same value: the address of the static buffer of the inet_ntoa() function. What inet_ntoa() does is declare a 'static char[SOME_SIZE_VALUE]', put the dots-and-numbers string in there, and return a pointer to its static buffer. (This is only even safe, BTW, because of the static-ness; you can't properly return a pointer to a stack-allocated variable.) You need to make your own copy of the string data, or else it just gets overwritten with the next call.

This is why we use std::string, and wrap ugly C interfaces as tightly as possible. :)
Ok, a data breakpoint should help track it down. If you're using VC2005 (2008 and 2003 should be similar), you can set a breakpoint in your code after the vector is intially set up, and then when the breakpoint is hit, go to Debug -> New Breakpoint -> New Data Breakpoint, and enter the address of the first element in the vector (Which you can see in the debugger).
Alternatively, step through the code and see when the line changes. If you're not familiar with Visual Studios debugger, see Superpig's excelent article.
Quote:Original post by Zahlman
Quote:Original post by dphoenix
This is the function to get myIP:

const char* Messenger::getMyIP(){    hostent* localInfo = gethostbyname("");  // get the local host name    struct in_addr addr;    memcpy(&addr,localInfo->h_addr_list[0],sizeof(struct in_addr));    const char* localIP = inet_ntoa(addr);    return localIP;}


I've confirmed that that part works.


Here's the thing: it doesn't.

First of all, the function is deprecated, as noted there. But more importantly,

Quote:inet_ntoa() returns the dots-and-numbers string in a static buffer that is overwritten with each call to the function.


(boldface original, italic emphasis mine.)

Messenger::getMyIP() actually always returns the same value: the address of the static buffer of the inet_ntoa() function. What inet_ntoa() does is declare a 'static char[SOME_SIZE_VALUE]', put the dots-and-numbers string in there, and return a pointer to its static buffer. (This is only even safe, BTW, because of the static-ness; you can't properly return a pointer to a stack-allocated variable.) You need to make your own copy of the string data, or else it just gets overwritten with the next call.

This is why we use std::string, and wrap ugly C interfaces as tightly as possible. :)


Wow...okay. So let me make sure I understand what happened. The address in the battleList that is being replaced is the other guy's address with mine, so I'm always sending myself messages but not the other guy. So basically, using the inet_ntoa() function overwrote his address with mine?

I guess I'd better go back and change the battleList to strings!

I'll make that change and then check back in with my results, if you guys don't mind.

Thanks!
Quote:Original post by Evil Steve
Ok, a data breakpoint should help track it down. If you're using VC2005 (2008 and 2003 should be similar), you can set a breakpoint in your code after the vector is intially set up, and then when the breakpoint is hit, go to Debug -> New Breakpoint -> New Data Breakpoint, and enter the address of the first element in the vector (Which you can see in the debugger).
Alternatively, step through the code and see when the line changes. If you're not familiar with Visual Studios debugger, see Superpig's excelent article.


Darn, see this is a time when it would be nice to be able to use Visual Studio. By far the best debugger I've used. I don't think Code::Blocks has that functionality (though I may be wrong!)
Quote:Original post by dphoenix
Wow...okay. So let me make sure I understand what happened. The address in the battleList that is being replaced is the other guy's address with mine, so I'm always sending myself messages but not the other guy. So basically, using the inet_ntoa() function overwrote his address with mine?

I guess I'd better go back and change the battleList to strings!

I'll make that change and then check back in with my results, if you guys don't mind.

Thanks!
Yup. You can pretty much guarantee that if a function returns a pointer, it's a pointer to temporary storage. There's some exceptions to that though (strdup() springs to mind).

I always found it useful to think of pointers like postal addresses and buildings. You have a block of memory allocated somewhere (Say 32 bytes inside the inet_ntoa function), and you have a much smaller (4 bytes on a 32-bit OS) pointer that stores the address of that block of memory (Much like a postal address of a few letters refers to a large building made of hundreds of bricks). When you return a pointer, you're returning the address of the chunk of memory, not a copy of it. Likewise, you can copy the pointer as much as you like, but it still points to the same address - you can have 50 envelopes addressed to the same address, you don't have 50 copies of the building.
Two calls to inet_ntoa() will cause the second call to overwrite the contents of the first. Going back to the anology, you can write a letter addressed to a building, rebuild the building, and then write another letter addressed to the same building (I.e. store a pointer to a buffer, change the buffer, and store another pointer - or call inet_ntoa(), store the result, then call inet_ntoa() again). The first letter will now be delivered to the new building that's at the same address. Likewise, after the second call to inet_ntoa(), the second pointer is the same as the first.

Anyway, I'm blithering. I'll shut up now.


EDIT:
Quote:Original post by dphoenix
Darn, see this is a time when it would be nice to be able to use Visual Studio. By far the best debugger I've used. I don't think Code::Blocks has that functionality (though I may be wrong!)
If you're developing for Windows, I'd strongly recommend getting Visual Studio - it's free and fully functional. You can always develop code using both Visual Studio and another IDE/compiler if you want to support multiple platforms.
You guys rocked it! I changed my const char* all to strings, and when I need to send a message I just used c_str(). It worked!

Funny thing is, Zahlman I've read that siggy of yours many times and it never really sank in till just now lol.

Steve, I actually do have Visual Studio, and I absolutely love the debugger. Only thing is for the project I'm working on, the networked code is straight from Linux libraries (I'm using Cygwin), and it won't compile on VS :( And you weren't blithering, I think you explained pointers well! I just had no idea inet_ntoa used the same address space each time...it never even occured to me, but it's like "duh" now ;)
Quote:Original post by dphoenix
Steve, I actually do have Visual Studio, and I absolutely love the debugger. Only thing is for the project I'm working on, the networked code is straight from Linux libraries (I'm using Cygwin), and it won't compile on VS :( And you weren't blithering, I think you explained pointers well! I just had no idea inet_ntoa used the same address space each time...it never even occured to me, but it's like "duh" now ;)
Plain BSD sockets should work fine in Visual Studio; you should just need to add calls to WSAStartup() and WSACleanup(). If you have other code though, it may be harder to port. If you can get the code to work on Visual Studio without too much tinkering, I'd highly recommend it. Visual Studio is widely used in the games industry, and it's debugger is the best you can get when developing on Windows.

There's also DevCpp which I gather has a debugger, but I've never actually used it myself, and I have no idea how well it plays with Cygwin (if at all).
Quote:Original post by dphoenix
You guys rocked it! I changed my const char* all to strings, and when I need to send a message I just used c_str(). It worked!

Funny thing is, Zahlman I've read that siggy of yours many times and it never really sank in till just now lol.


Yeah yeah, some people just have to push the big red button before they learn no matter what you tell them ;)

@Evil Steve: They often also return one of the parameters (if it was the same type), one of the parameters plus an offset (e.g. to indicate a position in an input string), or NULL. :)

This topic is closed to new replies.

Advertisement