Solved

Crashing in a char array loop c++

Posted on 2011-02-23
12
415 Views
Last Modified: 2012-05-11
Hello,
This is my code:
void SteamID()
{
	FILE * pFile;
  long lSize;
  char * buffer;
  size_t result;

  pFile = fopen ( "c:\\program files\\steam\\steam.log" , "rb" );
  if (pFile==NULL) {fputs ("File error",stderr); exit (1);}

  fseek (pFile , 0 , SEEK_END);
  lSize = ftell (pFile);
  rewind (pFile);

  buffer = (char*) malloc (sizeof(char)*lSize);
  if (buffer == NULL) {fputs ("Memory error",stderr); exit (2);}

  result = fread (buffer,1,lSize,pFile);
  if (result != lSize) {fputs ("Reading error",stderr); exit (3);}

  fclose (pFile);
  char * pch;
  char finalsid[256];
  int i = 0;
  pch = strstr (buffer,"for ");
  //cout << pch;
  for(i = 0; i < 256; ++i)
  {
	if(pch[i] == '\n')
	{
		pch[i+1] = '\0';
		break;
	}
	else
	{
		
		sprintf(finalsid, "%s", pch[i]);
	}
  }
  //cout << finalsid;
}

Open in new window



I'm trying to read from a steam log file, using strstr to take me near the steam ID, which I want to extract from the text. I then proceed to loop through the return of the strstr byte by byte, checking for the newline (and hence the end of the steam ID). and when a new line is reached, I put a null byte after it and write the bytes before the new line to a string.

some where in here I'm making a big mistake, because it's crashing; debug reports back an "Access Violation" which I'm unable to spot.

Any help;
Thanks.
0
Comment
Question by:JoeD77
12 Comments
 
LVL 45

Expert Comment

by:Kdo
ID: 34967167
Hi Joe,

My guess is that it's in lines 25 - 27.

What happens if strstr() returns NULL?


Good Luck,
Kent
0
 
LVL 16

Expert Comment

by:sjklein42
ID: 34967188
Several observations.

When you don't see a newline, and you do

sprintf(finalsid, "%s", pch[i]);

Open in new window


do you mean %c instead of %s?

Also, are you trying to build up the output string in fialsid by concatenation?  That's not what you're doing - you're just capturing the last character before the newline.   But that may not be relevant, because...

Shouldn't line 40 be

cout << pch;

Open in new window


Doesn't the stream ID string start at pch and end at the null byte you inserted after the newline?  I don't see why you need the finalsid variable at all.
0
 
LVL 32

Accepted Solution

by:
phoffric earned 500 total points
ID: 34967244
Possibly in line 15, you need to allocate one extra char for the terminating null byte and then set that last char to 0. Since you are dealing with c-style strings, then you should include the terminating null byte. At line 37, if not null terminated, then you could easily try filling the finalsid target overflowing it, or exceed virtual address space associated with your buffer as the sprintf function scans it copying everything into finalsid until it finds a 0 byte.

This question could likely be easily answered if you run your program in the VS 2010 debugger. In this particular program you will get an Unhandled Exception, and you can then hit the Break button. You should then see the line of code where the problem is and look at variables to see what the problem is.

If you haven't already done so, you can download the free Visual Studio Expresss C++ 2010:
    http://www.microsoft.com/express/Downloads/

I like the VS C++ debugger so much that I wrote articles on it. To quickly get started (about 15 minutes learning curve), you can read:

   C/C++ Beginner's Debugging Guide

After becoming familiar with the basics, move onto these two articles:
   Breakpoint Tips for C/C++

   Watch, Memory, Stack Tips: C/C++

0
 
LVL 32

Expert Comment

by:phoffric
ID: 34967261
You could also consider a rewrite using C++ string class. This tends to eliminate many access errors.
    http://www.cplusplus.com/reference/string/string/

But I guess that's another question for another day.
0
 

Author Comment

by:JoeD77
ID: 34967355
Thanks a lot for all the help

I changed line from:
 
for(i = 0; i < 256; ++i)
  {

	if(pch[i] == '\n')
	{
		break;
	}
	else
	{
		
		cout << finalsid;
		sprintf(finalsid, "%s", pch[i]);
		
		
	}
  }

Open in new window

 

to:

 
 for(i = 0; i < 256; ++i)
  {

	if(pch[i] == '\n')
	{
		break;
	}
	else
	{
		
		cout << finalsid;
		sprintf(finalsid, "%c", pch[i]);
		
		
	}
  }
  

Open in new window

(changed %s to %c)

and that solved the crash.

now it outputs:
¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦
¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦
¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦
¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦¦+¿|for 0:1:4075554

now that last bit of information on the end is what I need! (for 0:1:4075554)

I know why it's outputting all this other junk (array is 256 bytes) but I don't know how to accurately define the size of the array based on # of bytes that were checked before reaching the new line.

any help! thanks!

0
 
LVL 32

Expert Comment

by:phoffric
ID: 34967360
Did you add the terminating null byte as discussed?
0
How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

 
LVL 32

Expert Comment

by:phoffric
ID: 34967367
reverse lines 11 and 12
0
 

Author Comment

by:JoeD77
ID: 34967406
  char * pch;
  char finalsid[256] = {' \0 '};
  int i = 0;

  pch = strstr (buffer,"for ");
  for(i = 0; i < 256; ++i)
  {

	if(pch[i] == '\n')
	{
		break;
	}
	else
	{
		cout << finalsid;
		sprintf(finalsid, "%c", pch[i]);

		
		
	}
  }
  

Open in new window


Thanks for reminding me on the null byte :)

now it outputs fine, " for 0:1:4075554"

however, when I switch up the lines:
		cout << finalsid;
		sprintf(finalsid, "%c", pch[i]);

Open in new window


TO:
sprintf(finalsid, "%c", pch[i]);
		cout << finalsid;

Open in new window


it outputs nothing at all. I assume this is because the last byte being added to finalsid is a null byte from sprintf(). is there another way I should be concatenating that string?

Thanks!
0
 
LVL 32

Expert Comment

by:phoffric
ID: 34967434
strcat is the usual way to concatenate two null terminated strings
    http://www.cplusplus.com/reference/clibrary/cstring/strcat/

strncat is a little better in preventing overflows.
     http://www.cplusplus.com/reference/clibrary/cstring/strncat/

Keep in mind that pch[ i ] is a char and not a string.

re:     cout << finalsid;  
         sprintf(finalsid, "%c", pch[ i ]);
the first time through, finalsid has not been set to anything, so you should get garbage (unless you initialized it to 0 - then the cout yields nothing on the first pass).
0
 

Author Comment

by:JoeD77
ID: 34967514
  char * pch;
  char finalsid[256] = {' \0 '};
  int i = 0;

  pch = strstr (buffer,"for ");
  for(i = 0; i < 256; ++i)
  {

	if(pch[i] == '\n')
	{
		break;
	}
	else
	{

		strcat(finalsid, (const char*)pch[i]);

		
		
	}
  }

Open in new window


it crashes now with strcat()

:(!
0
 
LVL 32

Expert Comment

by:phoffric
ID: 34967541
Well, we at least fixed your OP crash.
pch[ i ] is a char, aka, an integer that sometimes represents an ASCII char, sometimes just a number.

You converted it to a pointer. So, no surprise that your ASCII char representation when treated as a memory address caused it to crash.

We did solve you OP crash question though.

Now, pch is a pointer. But if this does not help, then please ask this strcat issue as a separate question (per EE question policy).
0
 

Author Closing Comment

by:JoeD77
ID: 34967631
Ok that's fair. Thanks for reminding me of that policy. Thanks for all your help.
0

Featured Post

How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

Join & Write a Comment

Have you thought about creating an iPhone application (app), but didn't even know where to get started? Here's how: ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ Important pre-programming comments: I’ve never tri…
This article shows you how to optimize memory allocations in C++ using placement new. Applicable especially to usecases dealing with creation of large number of objects. A brief on problem: Lets take example problem for simplicity: - I have a G…
The goal of this video is to provide viewers with basic examples to understand and use switch statements in the C programming language.
The viewer will learn how to clear a vector as well as how to detect empty vectors in C++.

760 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

18 Experts available now in Live!

Get 1:1 Help Now