fclose(f_ptr2) - "Access Violation or memery can not be read"
-
Hi, [Added] ignore this reply, it is wrong! [/added] there is a problem in
fileName[i]=FindFileData.cFileName;this line does NOT copy the filename, it copies the pointer to the cFileName field in your unique FindFileData struct, hence in all iterations it will point to the buffer containing the last data written into it. If you want to hold all the different filenames, you must copy them, which you could do with strcpy() or strncpy(). BTW: your NULL test shows a MessageBox but then continues the program execution, which will result in failure of fscanf and/or fclose. The right way to handle this is to have an if-then-else with all file actions (fscanf/fclose) in one part, and the error handling (I do not really like MessageBox !) in the other part. :) -- modified at 13:39 Monday 26th November, 2007Luc Pattyn [Forum Guidelines] [My Articles]
this months tips: - before you ask a question here, search CodeProject, then Google - the quality and detail of your question reflects on the effectiveness of the help you are likely to get - use PRE tags to preserve formatting when showing multi-line code snippets
Luc Pattyn wrote:
there is a problem in fileName[i]=FindFileData.cFileName; this line does NOT copy the filename, it copies the pointer to the cFileName field in your unique FindFileData struct, hence in all iterations it will point to the buffer containing the last data written into it. If you want to hold all the different filenames, you must copy them, which you could do with strcpy() or strncpy().
Not necessary at all. The statement is correct, since
CStringhas an assignment operator that internally does the copying.fileName[0],fileName[1],fileName[2], etc will each contain different data.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
-
Luc Pattyn wrote:
there is a problem in fileName[i]=FindFileData.cFileName; this line does NOT copy the filename, it copies the pointer to the cFileName field in your unique FindFileData struct, hence in all iterations it will point to the buffer containing the last data written into it. If you want to hold all the different filenames, you must copy them, which you could do with strcpy() or strncpy().
Not necessary at all. The statement is correct, since
CStringhas an assignment operator that internally does the copying.fileName[0],fileName[1],fileName[2], etc will each contain different data.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
My mistake, was looking at it as a C function. :rolleyes:
Luc Pattyn [Forum Guidelines] [My Articles]
this months tips: - before you ask a question here, search CodeProject, then Google - the quality and detail of your question reflects on the effectiveness of the help you are likely to get - use PRE tags to preserve formatting when showing multi-line code snippets
-
While it (probably) has nothing to do with your problem, I'd offer: 1) Don't post commented-out code. It just makes that much more for us to have to read/ignore. 2) Since you are using MFC, why not take advantage of
CStdioFile,AfxMessagBox(), andCFileFind? That said, do the first 2-4 "columns" in your input file contain more than 14 characters? If so,bufferwill not hold them all. If there are more than 2000 files in the folder pointed to byDirSpec, you'll have obvious trouble. Your very lastfor()loop is usingiiandII. Is that intentional?mrby123 wrote:
DirSpec = (LPTSTR) malloc (BUFSIZE); DirSpec=TEXT("*.txt");
The address assigned to
DirSpec(frommalloc()) has been changed, and a subsequent call tofree()would fail. Consider:CStringArray fileNames;
CFileFind fileFind;BOOL bFound = fileFind.FindFile("*.txt");
while (bFound)
{
bFound = fileFind.FindNextFile();
fileNames.Add(fileFind.GetFilePath());
}fileFind.Close();
for (int ii = 0; ii < fileNames.GetSize(); ii++)
{
CString fileName = fileNames.GetAt(ii);CStdioFile fileIn; if (fileIn.Open(fileName, CFile::modeRead)) { CString line; fileIn.ReadString(line); // parse line here fileIn.Close(); }}
CStdioFile fileOut;
if (fileOut.Open(m_outputFileName, CFile::modeWrite))
{
CString str;str.Format("%d\\n", fileNames.GetSize()); fileOut.WriteString(str); fileOut.WriteString(str); fileOut.WriteString("fineName Easting(m) Northing(m) elv(m)\\n"); for (int ii = 0; ii < fileNames.GetSize(); ii++) { CString fileName = fileNames.GetAt(ii); str.Format("%s %f %f %f\\n", fileName, east\[ii\], north\[ii\], elv\[ii\]); fileOut.WriteString(str); } fileOut.Close();}
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Lau
DavidCrow wrote:
If so, buffer will not hold them all.
And worse, the data may spill and overwrite whatever is adjacent to buffer, causing unpredictable errors. Overwriting a pointer is likely to result in "Access violation". The present code is unsafe. :)
Luc Pattyn [Forum Guidelines] [My Articles]
this months tips: - before you ask a question here, search CodeProject, then Google - the quality and detail of your question reflects on the effectiveness of the help you are likely to get - use PRE tags to preserve formatting when showing multi-line code snippets
-
While it (probably) has nothing to do with your problem, I'd offer: 1) Don't post commented-out code. It just makes that much more for us to have to read/ignore. 2) Since you are using MFC, why not take advantage of
CStdioFile,AfxMessagBox(), andCFileFind? That said, do the first 2-4 "columns" in your input file contain more than 14 characters? If so,bufferwill not hold them all. If there are more than 2000 files in the folder pointed to byDirSpec, you'll have obvious trouble. Your very lastfor()loop is usingiiandII. Is that intentional?mrby123 wrote:
DirSpec = (LPTSTR) malloc (BUFSIZE); DirSpec=TEXT("*.txt");
The address assigned to
DirSpec(frommalloc()) has been changed, and a subsequent call tofree()would fail. Consider:CStringArray fileNames;
CFileFind fileFind;BOOL bFound = fileFind.FindFile("*.txt");
while (bFound)
{
bFound = fileFind.FindNextFile();
fileNames.Add(fileFind.GetFilePath());
}fileFind.Close();
for (int ii = 0; ii < fileNames.GetSize(); ii++)
{
CString fileName = fileNames.GetAt(ii);CStdioFile fileIn; if (fileIn.Open(fileName, CFile::modeRead)) { CString line; fileIn.ReadString(line); // parse line here fileIn.Close(); }}
CStdioFile fileOut;
if (fileOut.Open(m_outputFileName, CFile::modeWrite))
{
CString str;str.Format("%d\\n", fileNames.GetSize()); fileOut.WriteString(str); fileOut.WriteString(str); fileOut.WriteString("fineName Easting(m) Northing(m) elv(m)\\n"); for (int ii = 0; ii < fileNames.GetSize(); ii++) { CString fileName = fileNames.GetAt(ii); str.Format("%s %f %f %f\\n", fileName, east\[ii\], north\[ii\], elv\[ii\]); fileOut.WriteString(str); } fileOut.Close();}
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Lau
-
David, You solve my problem. You are right that I have a test which is longer than 15 characters in the data file. Thanks alots. Ruilin
You could have possibly found the problem sooner by using the debugger to step over each of the calls to
fscanf()and watch the value off_ptr2. I suspect it was changed by the timefclose()was called.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
-
You could have possibly found the problem sooner by using the debugger to step over each of the calls to
fscanf()and watch the value off_ptr2. I suspect it was changed by the timefclose()was called.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
DavidCrow wrote:
You could have possibly found the problem sooner by using the debugger
Surely YOU jest ;P
Mark Salsbery Microsoft MVP - Visual C++ :java:
-
DavidCrow wrote:
You could have possibly found the problem sooner by using the debugger
Surely YOU jest ;P
Mark Salsbery Microsoft MVP - Visual C++ :java:
No really, I'm dead serious. Science has proven that the debugger really does add years to your life, make you look taller, help you find problems quicker.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
-
Please check the code for me. Thanks a lots.
void CBlast_vib_procDlg::Next()
{
FILE *f_ptr1;
FILE *f_ptr2;
const int MAX=15;CBlast\_vib\_procDlg rr;//
float north[2000];
float east[2000];
float elv[2000];
CString fname[2000];
//
char buffer[MAX];// TODO: Add your control notification handler code here UpdateData(); if(m\_outputFileName == "" ) { MessageBox("All file names have to be typed in !"); rr.m\_outputFileName = m\_outputFileName; rr.DoModal(); UpdateData(false); } // TODO: Add extra validation here #define BUFSIZE MAX\_PATH WIN32\_FIND\_DATA FindFileData; LPTSTR DirSpec;//
HANDLE hFind = INVALID_HANDLE_VALUE;
CString fileName[2000];
int i=0;
DirSpec = (LPTSTR) malloc (BUFSIZE);
DirSpec=TEXT("*.txt");
//
hFind = FindFirstFile(DirSpec, &FindFileData);
fileName[0]=FindFileData.cFileName;
//
while (FindNextFile(hFind, &FindFileData) != 0)
{
i+=1;
fileName[i]=FindFileData.cFileName;
}
int nfile=i;
FindClose(hFind);
///*
//
for (int ii=0;iiI think this statement might have the problem. fscanf( f_ptr2,"%s %s %s\n",buffer,buffer,fname[ii].GetBuffer(MAX_PATH)); Here one of the pointer is taken using GetBuffer of CString. I think it is not the correct way of geting the pointer of CString memory and copying value in it like character array. Taking the value in character array and then assigning it to CString might be better option. Something like this. char sTemp[500]; fscanf( f_ptr2,"%s %s %s\n",buffer,buffer, sTemp); fname[ii] = sTemp;
-
I think this statement might have the problem. fscanf( f_ptr2,"%s %s %s\n",buffer,buffer,fname[ii].GetBuffer(MAX_PATH)); Here one of the pointer is taken using GetBuffer of CString. I think it is not the correct way of geting the pointer of CString memory and copying value in it like character array. Taking the value in character array and then assigning it to CString might be better option. Something like this. char sTemp[500]; fscanf( f_ptr2,"%s %s %s\n",buffer,buffer, sTemp); fname[ii] = sTemp;
Sunil Shindekar wrote:
I think this statement might have the problem. fscanf( f_ptr2,"%s %s %s\n",buffer,buffer,fname[ii].GetBuffer(MAX_PATH));
While it's awkward looking, there is nothing wrong with it.
Sunil Shindekar wrote:
Taking the value in character array and then assigning it to CString might be better option.
Different? Yes. Better? No.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
-
Sunil Shindekar wrote:
I think this statement might have the problem. fscanf( f_ptr2,"%s %s %s\n",buffer,buffer,fname[ii].GetBuffer(MAX_PATH));
While it's awkward looking, there is nothing wrong with it.
Sunil Shindekar wrote:
Taking the value in character array and then assigning it to CString might be better option.
Different? Yes. Better? No.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
GetBuffer returns the pointer to the memory which is enough to store the current string assigned to the CString object. You are using the same pointer to read the data from the file. If the data is too large to store in the currently allocated memory for the pointer by the CString, then there will be memory overrun. It can cause overwriting the values of other memory locations which may or may not include file pointer also or FILE structure also.
-
GetBuffer returns the pointer to the memory which is enough to store the current string assigned to the CString object. You are using the same pointer to read the data from the file. If the data is too large to store in the currently allocated memory for the pointer by the CString, then there will be memory overrun. It can cause overwriting the values of other memory locations which may or may not include file pointer also or FILE structure also.
Sunil Shindekar wrote:
If the data is too large to store in the currently allocated memory for the pointer by the CString, then there will be memory overrun. It can cause overwriting the values of other memory locations which may or may not include file pointer also or FILE structure also.
Aand how is your suggestion of using
char sTemp[500]any better?
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne