strcat crashes program

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

Recommended Posts

I am working on a program that downloads a web page source, I want to put all the source in one long string. unfortunatly it seems I have to call internetreadfile several times to get a whole web page, so I was trying to figure out how to get this to work and ended up finding strcat in the microsoft VC++ docs and thought I'd found the answer to my problems. I tried puting strcat into my program and compiled it and ran it. and nothing. the mouse changes to indicate that the program is starting up and then nothing, the program is not in memory (ctrl + alt + del, under task manager, no trace in either the applications or processes tabs) remarking out strcat fixes the problem. I've got a sneaking hunch that this is a stupid problem that I should know the answer to, but I did try a few things to fix it, substituting strcat for strncat, looking in the string.h file for the strcat function (couldn't find it) searching google for the same problem (found the problem but no solution) heres a code snippet

CHAR     szBuffer[1024];
CHAR     szWebPage[4000];
int      iTest;

while ( iTest = InternetReadFile( hFile, szBuffer, 1000, &dwRead )  )
{

if ( dwRead == 0 )
break;
strcat ( szWebPage, szBuffer}
}


heres the whole thing
#include <windows.h>
#include <stdio.h>
#include <tchar.h>
#include <wchar.h>
#include <string.h>
#include "stdafx.h"

LRESULT CALLBACK WndProc (HWND, UINT, WPARAM, LPARAM) ;

int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance,
PSTR szCmdLine, int iCmdShow)
{
static TCHAR szAppName[] = TEXT ("TestProgram") ;
HWND         hwnd ;
MSG          msg ;
WNDCLASS     wndclass ;

wndclass.style         = CS_HREDRAW | CS_VREDRAW ;
wndclass.lpfnWndProc   = WndProc ;
wndclass.cbClsExtra    = 0 ;
wndclass.cbWndExtra    = 0 ;
wndclass.hInstance     = hInstance ;
wndclass.hIcon         = LoadIcon (NULL, IDI_APPLICATION) ;
wndclass.hCursor       = LoadCursor (NULL, IDC_ARROW) ;
wndclass.hbrBackground = (HBRUSH) GetStockObject (WHITE_BRUSH) ;
wndclass.lpszMenuName  = NULL ;
wndclass.lpszClassName = szAppName ;

if (!RegisterClass (&wndclass))
{
MessageBox (NULL, TEXT ("Program requires Windows NT!"),
szAppName, MB_ICONERROR) ;
return 0 ;
}

hwnd = CreateWindow (szAppName, TEXT ("Test Program"),
WS_OVERLAPPEDWINDOW,
CW_USEDEFAULT,
CW_USEDEFAULT,
700,
150,
NULL, NULL, hInstance, NULL) ;

ShowWindow (hwnd, iCmdShow) ;
UpdateWindow (hwnd) ;

while (GetMessage (&msg, NULL, 0, 0))
{
TranslateMessage (&msg) ;
DispatchMessage (&msg) ;
}
return msg.wParam ;
}

LRESULT CALLBACK WndProc (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
{
static int  cxChar, cxCaps, cyChar ;
static int  cxClient, cyClient ;
HDC         hdc ;
PAINTSTRUCT ps ;
TEXTMETRIC  tm ;

// these are for placment calculations
static unsigned short int WSymbol, WShares, WWorth, WChange, WTotal;

switch (message)
{

case WM_CREATE:
hdc = GetDC (hwnd) ;

GetTextMetrics (hdc, &tm) ;
cxChar = tm.tmAveCharWidth ;
cxCaps = (tm.tmPitchAndFamily & 1 ? 3 : 2) * cxChar / 2 ;
cyChar = tm.tmHeight + tm.tmExternalLeading ;

WSymbol = 3 * cxCaps;
WShares = 11 * cxCaps;
WWorth  = 19 * cxCaps;
WChange = 29 * cxCaps;
WTotal  = 45 * cxCaps;

HINTERNET hINet, hFile;

hINet = InternetOpen("testap", INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, 0 );

CHAR     szBuffer[1024];
CHAR     szWebPage[4000];
CHAR     szTemp[64];
int      iTest;

if ( !hINet )
{
MessageBox ( hwnd, "InternetOpen Failed", "Failure", 0);
return 0;
}

hFile = InternetOpenUrl( hINet, "http://www.google.com", NULL, 0, 0, 0 ) ;

if ( hFile )
{

while ( iTest = InternetReadFile( hFile, szBuffer, 1000, &dwRead )  )
{

if ( dwRead == 0 )
break;
strcat ( szWebPage, szBuffer);

}
InternetCloseHandle( hFile );
}
InternetCloseHandle( hINet );

ReleaseDC (hwnd, hdc) ;

wsprintf ( szTemp, " %i characters where read", dwRead);

MessageBox ( hwnd, szTemp, "char's read", 0);
if (iTest)
MessageBox ( hwnd, "success!", "hurra", 0);

return 0 ;

case WM_SIZE:
cxClient = LOWORD (lParam) ;
cyClient = HIWORD (lParam) ;
return 0 ;

case WM_PAINT:
hdc = BeginPaint (hwnd, &ps) ;

EndPaint (hwnd, &ps) ;
return 0 ;

case WM_DESTROY:
PostQuitMessage (0) ;
return 0 ;
}
return DefWindowProc (hwnd, message, wParam, lParam) ;
}



Share on other sites
For debugging, check out std::string. See if the module functions correctly.

Kuphryn

Share on other sites
While I can't comment on your specific problem, I can say that your function is horribly broken. You can perform at most four successful reads before your application will bitch out. Or maybe it won't, it is hard to say. Maybe your application just won't run at all, which is the behavior you're seeing.

You need to keep track of how many characters you've put into szWebPage to ensure it isn't more than 4000. That isn't much [ignoring whitespace, ~800 words in a standard document, less on a web page]. Once you clear 4000, you get a buffer overflow, and everything falls apart quite nicely.

Or look into std::string, which is considerably safer as it not only does that for you, it'll stretch itself out as long as it needs to be.

CM

Share on other sites
You're not initializing szWebPage. strcat assumes that your string is already null-terminated, and an uninitialized char array is anything but that. What's probably happening is that strcat is searching for the end of the string, it overshoots the bounds of the array, stomps on memory that's not yours, and then Windows kills the application because of that. Just add a simple

szWebPage[0] = '\0';

after the declaration of szWebPage.

Share on other sites
std:string webpage;

webpage.reserve(8192);//most pages are atleast 8 kbs large
here some pseudo code which works for text files if your want to read binary files you had do it a little bit different so you don t need the terminating \0s which might destroy the binary format

{
szbuffer[1023] = '\0';
webpage+=szbuffer;
}

Share on other sites
Here is your fix (basically what Aprosenf is saying). Replace this
		  CHAR     szWebPage[4000];
with this
		  CHAR     szWebPage[4000] = "";

Share on other sites
Quote:
 Original post by iMalcHere is your fix (basically what Aprosenf is saying). Replace this CHAR szWebPage[4000];with this CHAR szWebPage[4000] = "";

No, you can't assign a string literal to a char array like that, because the array is a char * const - you can't repoint it. You can certainly initialize the array (or at least make it appear to contain zero-length data) in the way that Aprosenf indicated, or initialize as "CHAR szWebPage[4000] = {0}", but either of these still has lots of problems.

I'll bet dollars to donuts that InternetReadFile does not null-terminate the read-in data, since it's reading into a "buffer", not a "string". This is indicated by the manual assignment to szBuffer in the original code (and reading only 1000 chars into a 1024 char buffer). Basiror has it correct, although I'd do it like:

const int KILOBYTE = 1024;const int CHUNKSIZE = KILOBYTE;const int MIN_EXPECTED_SIZE = 8 * KILOBYTE;CHAR buffer[CHUNKSIZE + 1];buffer[CHUNKSIZE] = '\0'; // make sure that any read-in data is null terminated// We only need to do that once since by design we don't want to overwrite herestd::string pageSource; pageSource.reserve(MIN_EXPECTED_SIZE);DWORD readErrorCode, amountRead;while ((readErrorCode = InternetReadFile(file, buffer, CHUNKSIZE, &amountRead))       && amountRead) {  pageSource += buffer;}

Share on other sites
Quote:
 Original post by ZahlmanNo, you can't assign a string literal to a char array like that, because the array is a char * const - you can't repoint it. You can certainly initialize the array (or at least make it appear to contain zero-length data) in the way that Aprosenf indicated, or initialize as "CHAR szWebPage[4000] = {0}", but either of these still has lots of problems.

CHAR szWebPage[4000] = ""; is a valid char array initialization statement, equivalent to CHAR szWebPage[4000] = {0};

Share on other sites
Quote:
Original post by SiCrane
Quote:
 Original post by ZahlmanNo, you can't assign a string literal to a char array like that, because the array is a char * const - you can't repoint it. You can certainly initialize the array (or at least make it appear to contain zero-length data) in the way that Aprosenf indicated, or initialize as "CHAR szWebPage[4000] = {0}", but either of these still has lots of problems.

CHAR szWebPage[4000] = ""; is a valid char array initialization statement, equivalent to CHAR szWebPage[4000] = {0};
Exactly, and one I use regularly enough too. It's one of the rare cases where you don't have to use a strxxx function. When you say "either of these still has lots of problems" do you mean with the rest of the code? I must admit, I wasn't looking for other bugs.

You've been using C++ strings too long[smile]. Oh well, that's all good anyway.

Share on other sites
Does
CHAR szWebPage[4000] = "";
really null all the 4000 bytes as
CHAR szWebPage[4000] = {0};
does?
EDIT: I tried it and it seems CHAR szWebPage[4000] = ""; really nulls the whole array. I expected only the length of the string literal to be copied over...

1. 1
2. 2
Rutin
23
3. 3
JoeJ
20
4. 4
5. 5
gaxio
13

• 24
• 40
• 23
• 13
• 13
• Forum Statistics

• Total Topics
631734
• Total Posts
3001933
×