Back to General and Gameplay Programming

strcat crashes program

General and Gameplay Programming Programming

Started by PPCThug August 03, 2005 08:35 PM

15 comments, last by Zahlman 18 years, 8 months ago

PPCThug

130

Author

August 03, 2005 08:35 PM

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;
					szBuffer[dwRead] = 0;
					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];
		  DWORD    dwRead;
		  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;
					szBuffer[dwRead] = 0;
					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) ;
}

Bloodshed Dev-C++ 4.9.8.0 Mingw DX 9.0a DX SDK 6.1win2k#define WIN32_LEAN_AND_MEANthe Particle Projection Cannon fires a shimmering blue bolt, much like a cross between lightning and a sine wave that ripples along its path.mechwarrior 2 mercenaries, 4 particle projection cannons, thug chassis

kuphryn

210

August 03, 2005 09:57 PM

For debugging, check out std::string. See if the module functions correctly.

Kuphryn

Conner McCloud

1,135

August 04, 2005 12:31 AM

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

Aprosenf

372

August 04, 2005 01:02 AM

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.

Basiror

241

August 04, 2005 01:24 AM

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

while(..internetreadfile(szbuffer, 1023...))
{
szbuffer[1023] = '\0';
webpage+=szbuffer;
}

http://www.8ung.at/basiror/theironcross.html

iMalc

2,466

August 04, 2005 04:28 AM

Here is your fix (basically what Aprosenf is saying). Replace this

		  CHAR     szWebPage[4000];

with this

		  CHAR     szWebPage[4000] = "";

"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms

Zahlman

1,682

August 04, 2005 02:03 PM

Quote:Original post by iMalc
Here 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;}

SiCrane

11,840

August 04, 2005 02:15 PM

Quote:Original post by Zahlman
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.

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

iMalc

2,466

August 04, 2005 02:26 PM

Quote:Original post by SiCrane
Quote:Original post by Zahlman
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.

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.

"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms

izhbq412

204

August 04, 2005 02:57 PM

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...

_______________The essence of balance is detachment. To embrace a cause, to grow fond or spiteful, is to lose one''s balance after which, no action can be trusted. Our burden is not for the dependent of spirit. - Mayar, Third Keeper

strcat crashes program

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

strcat crashes program

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines