?
Solved

Crashing in a char array loop c++

Posted on 2011-02-23
12
Medium Priority
?
450 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
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
12 Comments
 
LVL 46

Expert Comment

by:Kent Olsen
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 2000 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
What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

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

Free Tool: ZipGrep

ZipGrep is a utility that can list and search zip (.war, .ear, .jar, etc) archives for text patterns, without the need to extract the archive's contents.

One of a set of tools we're offering as a way to say thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

When writing generic code, using template meta-programming techniques, it is sometimes useful to know if a type is convertible to another type. A good example of when this might be is if you are writing diagnostic instrumentation for code to generat…
IntroductionThis article is the second in a three part article series on the Visual Studio 2008 Debugger.  It provides tips in setting and using breakpoints. If not familiar with this debugger, you can find a basic introduction in the EE article loc…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will learn additional member functions of the vector class. Specifically, the capacity and swap member functions will be introduced.

762 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