Thread: Stack around variable corrupted?

I think this probably belongs here, since it seems to be an error with my C++, not my interpretation of winsock. However, feel free to move it if I'm wrong.

Okay, when I run this program, it returns the webpage okay, but I've been having problems once it's finished; it exits with this debug error: Run-Time Check Failure #2 - Stack around the variable 'stuff' was corrupted.

Here's the improtant code (the rest is just a house for it)


greeting.cpp

Code:

#include "greeting.h"

//this logs the data into a text file
void WriteToFile(char *stuff)
{
ofstream current_file("greeting_log.html", ios::app); //open the file
current_file << stuff << "\n";
current_file.close();

}

void ReportError(int errorCode, const char *whichFunc)
{
char errorMsg[92]; //character array to hold the message
ZeroMemory(errorMsg, 92); //automatically null-terminate the string

//copy the phrase, whichFunc string, and int errorCode into the buffer

sprintf(errorMsg, "Call to %s returned error %d!", (char *)whichFunc, errorCode);
WriteToFile(errorMsg);
}




int greeting()

{
char website[60];

cout << "What website do you want? ";
cin >> website;


//variables
WORD sockVersion; //holds the version of winsock in use
WSADATA wsaData; //queries the version of winsock required
int nret;   //stores error stuff
char info[100];
char stuff[400];

sprintf(info, "<!--");
WriteToFile(info);

sockVersion = MAKEWORD(1,1); //we need winsock version 1.1

nret = WSAStartup(sockVersion, &wsaData); //initialise winsock

sprintf(info, "sockVersion = %ld", nret);
WriteToFile(info);

SOCKET theSocket; //my single socket

theSocket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); //initialises the socket

if(theSocket == NETWORK_ERROR)
{
 nret = WSAGetLastError();  //mroe detailed error info
 ReportError(nret, "connect()"); //reports error with custom function

 WSACleanup();  //clean up winsock
 return NETWORK_ERROR; //return -1
}
sprintf(info, "Socket = %ld", theSocket);
WriteToFile(info);

//host information (well, location)
   LPHOSTENT hostEntry;
hostEntry = gethostbyname(website);
sprintf(info, "hostEntry = %s", hostEntry);
WriteToFile(info);

//fill in socket addressing information
   SOCKADDR_IN serverInfo;

serverInfo.sin_family = AF_INET; //uses TCP/IP protocol suite
serverInfo.sin_addr = *((LPIN_ADDR)*hostEntry->h_addr_list); //works out all addresses of target
serverInfo.sin_port = htons(80); //port to connect through

sprintf(info, "server info filled in");
WriteToFile(info);

nret = connect(theSocket,(LPSOCKADDR)&serverInfo, sizeof(sockaddr));

if(nret == SOCKET_ERROR)
{
 nret = WSAGetLastError();  //mroe detailed error info
 ReportError(nret, "connect()"); //reports error with custom function

 WSACleanup();  //clean up winsock
 return NETWORK_ERROR; //return -1
}

sprintf(info, "Connect = %ld", nret);
WriteToFile(info);

strcpy(stuff, "GET / \r\n");
nret  = send(theSocket, stuff, sizeof(stuff) , 0);
sprintf(info, "send = %ld", nret);
WriteToFile(info);

while(nret != 0)
{
 strset(stuff, ' ');
 nret = recv(theSocket, stuff, sizeof(stuff), 0);
 WriteToFile(stuff);
}

MessageBox(0, "done!", "Finished", 0);

sprintf(info, "-->");
WriteToFile(info);


closesocket(theSocket);
WSACleanup();

return(0);
}

On a related note, how would I get stuff from subdirectories? I can only seem to get something from the front page of the website (ie. www.binrev.com). Cheers in advance!

This part looks strange to me

Code:

while(nret != 0) {
   strset(stuff, ' ');
   nret = recv(theSocket, stuff, sizeof(stuff), 0);
   WriteToFile(stuff);
}

I don't think that recv will terminate the received data with '\0'. So if you receive more then one block of data ( that seems to be the purpose of the while loop ) you call strset() on a non terminated string and strset() will write over the bounds of stuff. Think you must not use any string-functions in this part of the code (including Write to File() ) because you will not get any 0-termination from recv.
Kurt

strcpy(stuff, "GET / \r\n");
nret = send(theSocket, stuff, sizeof(stuff) , 0);
sprintf(info, "send = %ld", nret);
WriteToFile(info);

So you copy in 8 characters, then send 400 ?

Originally Posted by nvoigt

Sidenote:

Code:

(char *)whichFunc

Casting away a const modifier ( especially without any reason ) will get you tarred and feathered where I work.

Good point, I shouldn't have done that... consider it removed

Zuk - what do I do about it? I mean, surely I need to use it as a string in order to write it to a text file? The thing is, I can actually get the data, but I just get this error. So could I just tell it to WriteToFile() a \0 after the loop, or should I put it into the loop itself?

Originally Posted by timmygee

Zuk - what do I do about it? I mean, surely I need to use it as a string in order to write it to a text file? The thing is, I can actually get the data, but I just get this error. So could I just tell it to WriteToFile() a \0 after the loop, or should I put it into the loop itself?

The way you use WriteToFile() and strset() you should have a lot of spaces too many in your outputfile.
The best thing would be to open the file in binary mode and just write the nr of characters to the file that recv returns.
Like this

Code:

ofstream  = current_file("greeting_log.html", ios::binary);
while(nret != 0) {
   nret = recv(theSocket, stuff, sizeof(stuff), 0);
   current_file.write( stuff, nret);
}
current_file.close();

Ok that might not work exactly this way because you seem to write other things to that file as well. But you should get the idea.
If you still want to write the received data as string to the file you will have to concatenate the received blocks in memory using memcpy(), append the string terminator and use your WriteToFile() function to write the complete received data to the file.
Kurt

hmmm. but it is outputting into the textfile perfectly okay... I'm not at college right now, but once I get back, I'll try your modifications. That'll be on monday, so meh.

Originally Posted by timmygee

hmmm. but it is outputting into the textfile perfectly okay... I'm not at college right now, but once I get back, I'll try your modifications. That'll be on monday, so meh.

I cannot test and prove my statements because I do not sit at a windows-box. But what I understand from the documentation recv() does not 0-terminate the received data ( it must not because it can be used to receive binary data as well ). So your call to strset() will fill the memory with spaces until it finds a 0-byte somewhere. In your WriteToFile() function you use strlen() to get the length of the received data witch again just searches for a 0-byte. That's why I think you should have a lot of spaces in your outputfile. ( maybe you just don't see them they are kind of invisible )
Kurt