Jump to content
  • Advertisement
Sign in to follow this  
Jouei

Yet another Corrupted Buffer

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

If you intended to correct an error in the post then please contact us.

Recommended Posts

Ok here i am again with a corrupted buffer and for the life of me i couldnt figure out how i fixed it the last time the code below is used to load in numbers for a map each number represents a tile type ! hope someone can posibly tell me what the problem seems to be as when i try and run it the NumberBuffer seems to get corrupted! struct DRAWTILES { int TileNumber; }; int Paras(char *BufferTo,int StartPoint) { int Number = -1; int Length = strlen(BufferTo); if(StartPoint != 0) { StartPoint++; } int I = StartPoint; while(I <= Length) { if(BufferTo == ',') { Number = I; I = Length; } I++; } return Number; } DRAWTILES DrawTiles[20][15]; bool LoadMap(char *MapToLoad) { ifstream Map; char TempBuffer[200]; char NumberBuffer[3]; int GridWidth = 19; int GridCurX = 0; int GridHeight = 14; int GridCurY = 0; int CurParas = -1; int TempParas = 0; int TempParas2 = 0; int I = 0; Map.open(MapToLoad); if(!Map) { return false; } Map >>TempBuffer; while(GridCurY < 14) { CurParas = Paras(TempBuffer,CurParas); TempParas = Paras(TempBuffer,CurParas); TempParas2 = CurParas; TempParas2++; while(TempParas2 < TempParas) { NumberBuffer = TempBuffer[TempParas2]; I++; TempParas2++; } DrawTiles[GridCurX][GridCurY].TileNumber = atoi(NumberBuffer); GridCurX++; NumberBuffer[0] = ' '; NumberBuffer[1] = ' '; NumberBuffer[2] = ' '; NumberBuffer[3] = ' '; if(GridCurX >14) { GridCurX = 0; GridCurY++; } } MessageBox(hwnd,"Map Load Ok",NULL,MB_OK); return true; } i hope this dosent confuse anyone to much but thanks for any assitances Thanks ahead of time Jouei! Ps sry about the bad spelling im sure its in here somewhere!

Share this post


Link to post
Share on other sites
Advertisement
First of all, in the 'LoadMap' function, don't make 'TempBuffer' be a char array, make it a std::string, that way it will allocate enough memory itself and you won't have to worry about buffer overflow.


  • Choose a more descriptive name than 'Paras'. I have no idea what that could be.
  • Choose a better name than 'TempParas2'. It will be very confusing.
  • You never decrease the 'i' variable or set it back to 0. What if the nested 'while' is iterated over 4-5 times? You've stuffed your memory access.


I don't know if this is the problem, but it is definately A problem:

bool LoadMap(char *MapToLoad)
{
// snip
char NumberBuffer[3];

// snip

while(GridCurY < 14)
{
// snip
NumberBuffer[0] = ' ';
NumberBuffer[1] = ' ';
NumberBuffer[2] = ' ';
NumberBuffer[3] = ' ';
}

// snip
}




You are setting 4 elements of a char array, when you have allocated only 3. Also, atoi stops converting at the end of the string. Because you're using a C-style string, it will keep attempting to convert into a number until it hits the first element with an integer value of 0. Because you haven't set the end of the string yourself, you have no idea where it will stop, and it will very likely return absolute garbage.

Remember, a C-style string is only a char array, and is only considered a string if there is an integer value of 0 in one of the array elements.

Share this post


Link to post
Share on other sites
'ifstream' indicates you're using C++. Or at least, seem to think you are.

In C++, we represent text with the standard library class std::string(). We don't need to implement a "Paras" function (and noone here seems to have any idea what that's supposed to mean!), because std::string already provides search functionality (and even if it didn't, the standard library also provides generalized find-element-within-container functionality).

Also, reading with operator>> into a string or character buffer reads only one word of the input; I assume you want to read a line at a time. For this we use the function std::getline().

But it seems that what you're *really* trying to do is read from the file up to the next comma each time. Fortunately, std::getline() also covers this :)

Also, scope your variables appropriately: don't just declare a huge block of them at the beginning of a function. Prefer to initialize them with their first values where possible, and declare at or near first use.

Also, don't use atoi() to convert text to numbers. Instead, rely on the way that streams work: you can read directly in to an integer variable. Here, though, our goal is to pick out the commas first, and verify that each comma-separated item is a number, so what we will do is read comma-separated chunks and create temporary "string streams" from them, and try to read the numbers out of *those*.

Also, don't handle loops like that. Use for loops so that you don't have to do the incrementing/resetting yourself. Oh, and use named constants instead of magic numbers.

Also, use [code][/code] (for short segments) or [source][/source] (for longer ones) tags to format your code for posting here.


struct DRAWTILES {
int TileNumber;
};

const int MAP_WIDTH = 20;
const int MAP_HEIGHT = 15;

DRAWTILES DrawTiles[MAP_WIDTH][MAP_HEIGHT];

bool LoadMap(const std::string& name) {
std::ifstream Map(name.c_str());
if (!Map) {
return false;
}

string item;
for (int y = 0; y < MAP_HEIGHT; ++y) {
for (int x = 0; x < MAP_WIDTH; ++x) {
std::getline(Map, item, ',');
int number;
std::stringstream parser(item);
if (!(parser >> number)) {
return false; // corrupt file; no number there.
}
char c;
if (parser >> std::ws >> c) {
return false; // corrupt file; extra garbage.
}
DrawTiles[x][y].TileNumber = number;
}
}
MessageBox(hwnd, "Map Load Ok", NULL, MB_OK);
return true;
}



Yes, It's Really That Easy(TM).

Share this post


Link to post
Share on other sites
Sry i apologize i converted from C a long while back to C++
that whole string thing you have well kinda new to me lol
if you could give a rough outline as to how its used and its header file
i will just look up the rest on msdn.

PS i got the code working i apologize ahead of time for the incvenice of lacking information i gave prioer to the code. i was quite tired at the point in time.

anyways thanks for the information on the string stuff

have a good day and thanks ahead of time!

Share this post


Link to post
Share on other sites
Quote:
Original post by Jouei
if you could give a rough outline as to how its used and its header file
To compile Zahlman's code, you'll need the following includes:
#include <iostream>
#include <sstream>
#include <fstream>
#include <string>
along with
using std::string;
to take care of the unqualified instance of string.

Admiral

Share this post


Link to post
Share on other sites
Quote:
Original post by TheAdmiral
along with
using std::string;
to take care of the unqualified instance of string.

Admiral


Oh, I missed one ;) (I had assumed the OP was using namespace std; because of the unqualified 'ifstream' usage, but decided to qualify things for pedagogical purposes. Although I am suddenly filled with horror at the thought that the OP might have invoked the dread iostream.h :S)

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!