|
First of all, thank you for your great class. <3
I think in BOOL CDiskObject::RenameDirectory( const CString& oldDirectory, const CString& newName )
if( ::MoveFile( fullName, fullNewName ) )
{
SetSystemErrorMessage( ::GetLastError(),
fullName +
_T( " -> " ) +
fullNewName );
}
else
result = true;
should be
if( ::MoveFile( fullName, fullNewName ) )
{
result = true;
}
else
{
SetSystemErrorMessage( ::GetLastError(),
fullName +
_T( " -> " ) +
fullNewName );
}
because ::MoveFile returns nonzero if it succeeds.
Best regards
Oliver
|
|
|
|
|
I am using the Non MFC version.
Recently (today ) I have found a small bug in copyfile function.
Here is the original line 1885 in StdDiskObject.cpp :
int file2 = _topen( to, _O_WRONLY | _O_CREAT, _S_IREAD | _S_IWRITE | _O_BINARY );
The bug is in the third argument in the call to the function _topen .
| _O_BINARY should be part of the second argument.
The correct line 1885 should be as follows:
int file2 = _topen( to, _O_WRONLY | _O_CREAT | _O_BINARY, _S_IREAD | _S_IWRITE);
Further more, there is an additional thing to be changed.
Since, we want our file to be exact copy of the original, _O_TRUNC should be also part of the second argument in the same call to _topen . In case that the existing file is larger than the file we are copying, without _O_TRUNC the resulting file will be greater than the original file we copied over the existing file.
The final correct version of line 1885 should be as follows:
int file2 = _topen( to, _O_WRONLY | _O_CREAT | _O_TRUNC | _O_BINARY, _S_IREAD | _S_IWRITE);
|
|
|
|
|
Duly noted - thanks for the feedback! We'll see if this article gets updated, if so, I'll certainly include this correction.
|
|
|
|
|
Quick question. Are you doing a UNICODE build? I'm having some issues here, making a few changes.
Love this code. Couple of notes for unicode and threading support ... will add to this post as i find them.
int filehandle = _open( infile.c_str(), _O_RDONLY );
int filehandle;
_tsopen_s( &filehandle, infile.c_str(), _O_RDONLY, _SH_DENYNO, _S_IREAD | _S_IWRITE);
-----------------------
int filehandle = _tcreat( infile.c_str(), _S_IREAD | _S_IWRITE );
int filehandle;
_tsopen_s(&filehandle, infile.c_str(), _O_CREAT, _SH_DENYNO, _S_IREAD | _S_IWRITE );
if( str.length( ) < 2 || str.substr( 0, 2 ) != "\\\\" )
#ifdef _UNICODE
if( str.length( ) < 2 || (str.substr( 0, 2 ) != L"\\\\") )
#else
if( str.length( ) < 2 || (str.substr( 0, 2 ) != "\\\\") )
#endif
close( filehandle );
_close( filehandle );
_tsplitpath( str.c_str(), drive, dir, fname, ext );
_tsplitpath_s( str.c_str(), drive, dir, fname, ext );
Built with VS2008 SP1
There may be more .. still working on it
Thanks again .. great class.
modified on Friday, May 1, 2009 1:45 PM
|
|
|
|
|
Quick answer: I am not doing UNICODE builds.
Regardless of the build type, consider the following:
- wherever _O_CREAT is used, one should think whether _O_TRUNC should be also used or not.
- _S_IREAD | _S_IWRITE are needed (and are used) only in combination with _O_CREAT .
|
|
|
|
|
According to MSDN : http://msdn.microsoft.com/en-us/library/w64k0ytk(VS.80).aspx[^]
"_O_CREAT
Creates and opens new file for writing. Has no effect if file specified by filename exists."
I imagine you want a new file, _O_TRUNC is not required. You want to overwrite an existing file if it exist; _O_TRUNC flag destroys the contents of an existing file. Either way, wouldn't hurt. I could be wrong, I haven't exercised the part of the code you speak of in your original message.
|
|
|
|
|
First of all, thanks for sharing the great codes.
I am having a problem as subject said.
It is inside
bool CStdDiskObject::CreateDirectory( const tstring& directory )
at line:
directories.push_back( parts.substr( 0, find ) );
which was called by:
bool CStdDiskObject::CopyFile( const tstring& sourceFile,const tstring destDirectory )
I have downloaded the latest code from this article's link and still the same.
Any suggestions/advice?
Regards,
Ke
|
|
|
|
|
Ke,
Thanks for the feedback!
A dangling pointer is a pointer that is already free'd but continues to be used. Are you sure that in-parameter(s) to the CStdDiskObject call still is/are valid?
|
|
|
|
|
Hello Johan:
Thank you for your response.
I got the dangling pointer message from Bounds Checker. I am using VC6 sp5. Will have a look through this week.
Regards,
Ke
|
|
|
|
|
Why the horizontal scroolbars doesn't work?
|
|
|
|
|
As horizontal scrollbars is not part of the class, no idea
You have to set the extent of the scrollbar manually - see the docs for CListBox . As this is a demo app for the disk ops class, I've not taken care going through stuff like that.
|
|
|
|
|
Hi Johan,
At first thanks for sharing this nice class, I really like it!
Anyway, I found a small problem in EnumAllDirs() . Did you ever try to enumerate all files of the root folder of your system drive on a NTFS system?
This is the part that causes the problem:
BOOL CDiskObject::EnumAllDirs( const CString& sourceDirectory, CStringArray& directories )
{
...
HANDLE findhandle = ::FindFirstFile( sourceFiles, &ff );
if( findhandle != INVALID_HANDLE_VALUE )
{
BOOL res = TRUE;
while( res )
{
if( ( ff.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY ) && _tcscmp( ff.cFileName, _T( "." ) ) && _tcscmp( ff.cFileName, _T( ".." ) ) )
{
CString directory( source + ff.cFileName );
directories.Add( directory + _TCHAR( '\\' ) );
Trigger( ff.cFileName );
result = EnumAllDirs( directory, directories );
if( !result )
res = FALSE;
}
if( res )
res = ::FindNextFile( findhandle, &ff );
}
::FindClose( findhandle );
}
else
{
SetSystemErrorMessage( errno, sourceFiles );
result = false;
}
...
} Let me explain: ::FindFirstFile() fails when access denied directories are about to be processed. AFAIK denying access for directories is a NTFS specific feature so the problem should only occur on such systems. So if such a directory is processed the else branch is called which stops the enumeration process.
On my machine the problem occurs when "C:\System Volume Information" is processed and in consequence the call to EnumAllFiles("C:\", ...) returns only the files that are directly in the root directory.
So my quick and dirty hack would be the following:
BOOL CDiskObject::EnumAllDirs( const CString& sourceDirectory, CStringArray& directories )
{
...
HANDLE findhandle = ::FindFirstFile( sourceFiles, &ff );
if( findhandle != INVALID_HANDLE_VALUE )
{
...
}
else
{
if(::GetLastError() != 5)
result = false;
SetSystemErrorMessage( errno, sourceFiles );
} Can you think of any side effects of this change? Or do you perhaps have a smarter solution?
cheers,
mykel
OMM: "Let us be thankful we have an occupation to fill. Work hard, increase production, prevent accidents and be happy."
|
|
|
|
|
mykel wrote: Can you think of any side effects of this change? Or do you perhaps have a smarter solution?
This looks like an OK change. I'll actually add it if/when the article gets updated.
|
|
|
|
|
Hi, Johan:
Thank you very much for sharing with us the great codes!
I just tried your updated version of the DiskObject (the first downloadable file from this website named as:diskobject_src.zip)to create a file which doesn't exist at first. However, it always fails with a returned error message "Cannot create a file when that file already exists." You could be able to repeat the error by copying the two source files to your demo project: diskobjectdemo_src.zip to replace the old version and try the "create new directory" command.
I'm new to C++ programming thus cannot identify the problem. Could it be a small bug of the updated version?
FYI. My OS is Windows XP Pro and compiler used is Visual Studio.Net 2003 (VC++).
Again, thank you for your generosity of sharing with us your great codes and your continuous efforts to improve them.
Best regards,
-- modified at 4:31 Wednesday 26th July, 2006
|
|
|
|
|
There is indeed a bug, that I have fixed in the version used for my own projects. I'll update the article as soon as possible!
|
|
|
|
|
Why are all your strings passed in by value?
Why not use const CString& instead?
Any sufficiently gross incompetence is nearly indistinguishable from malice.
|
|
|
|
|
Very good question. Pick one of oversight, laziness or gross incompetence
Seriously, I'll put this on the todo-list.
|
|
|
|
|
I don't believe this makes much a difference as CString uses a shared memory location and copy on write (at least it did in VC 6 and lower) so in both cases you are essentialy passing the same thing (an object containing 32 bit pointer to shared memory). I am not sure if MSFT did away with this in .net so it might be important still...
John
|
|
|
|
|
Still, it's damned difficult to write const correct code if utility functions are not.
|
|
|
|
|
Hi Johan,
Thanks so much for your great articles, I have learned heaps from you in my attempts to learn c++ mfc programming.
Just to let you know, I needed to copy a file from a unc path i.e. \\server\share\file.txt, but found that the copy routine got confused and added a drive letter 'C:\\server\share\file.txt'. My rude but successful hack was to remark out the 'qualifypath' function on the source.
I'm not clever enough to change that function, all the _tchar stuff gives me a headache!
Stu
|
|
|
|
|
Thanks for the kind feedback!
There is in fact finally an update in the pipeline for this article, I've been neglecting my duties to the community far too long
|
|
|
|
|
the sourceline tstring sourcefile(source+file) produces dump when in source are spaces like "All Users" in my case this was shorten to "All" instead opf adding the file-content. All binary files I copied are corrupted after using this function.
-- modified at 11:57 Friday 12th May, 2006
|
|
|
|
|
First thanks for your great class!
I don't know whether this small bug was already reported or not, if so, please
ignore my following message:
BOOL CiDisk::CreateDirectory( CString directory )
{
.....
if( parts.GetLength() > 2 )
{
if( parts.Left( 2 ) == _T( "\\\\" ) )
{
// We have an UNC name
CString strComputer;
parts = parts.Right( parts.GetLength() - 2 );
int findDir = parts.Find( _T( "\\" ) );
if(findDir!=-1)
{
strComputer = _T( "\\\\" ) + parts.Left( findDir );
parts = parts.Right( parts.GetLength() - ( findDir + 1 ) );
}
//----------------------------------------
// rw: if the computer name exceeds _MAX_DRIVE the m_feedbackWindow
// member will be filled with an value!=NULL, which forces an assertion
// calling Trigger(a) next time.
//
_tcscpy( drive, strComputer );
//----------------------------------------
}
.....
}
and here my small workaround, seems to work (improvements are welcome):
BOOL CiDisk::CreateDirectory( CString directory )
{
.....
_TCHAR long_drive[ MAX_PATH ]; //rw: new
strcpy( long_drive, drive); //rw: new
if( parts.Left( 2 ) == _T( "\\\\" ) )
{
// We have an UNC name
CString strComputer;
parts = parts.Right( parts.GetLength() - 2 );
int findDir = parts.Find( _T( "\\" ) );
if(findDir!=-1)
{
strComputer = _T( "\\\\" ) + parts.Left( findDir );
parts = parts.Right( parts.GetLength() - ( findDir + 1 ) );
}
//----------------------------------------
// rw: if the computer name exceeds _MAX_DRIVE the m_feedbackWindow
// member will be filled with an value!=NULL which, forces an assertion
// calling Trigger(a) next time.
//
// _tcscpy( drive, strComputer );
_tcscpy( long_drive, strComputer ); //rw: new
//----------------------------------------
}
// CString strRoot( drive );
CString strRoot( long_drive ); //rw: new
......
}
Best regards and keep on your great work!
Ralph
|
|
|
|
|
Thanks for the feedback! This baby is in for an overhaul any day now
|
|
|
|
|
Hi John,
Hope you remember me!!! I had done some coding on the Flow chart editor for connecting the elements.
This is an excellent class and I am using it in one of my projects. There is a small BUG in EnumAllFilesWithFilter. You forgot to clear the dirfiles variable and the stringarray will contain duplicate entries.
One of the nested loops has to be modified like this
for( int t = 0 ; t < max1 ; t++ )
{
dirfiles.RemoveAll( ); // --newly added
CString dir = dirs[ t ];
QualifyPath( dir );
if( EnumFilesInDirectoryWithFilter( filter, dir, dirfiles, EF_FULLY_QUALIFIED ) )
{
int max2 = dirfiles.GetSize( );
for( int i = 0 ; i < max2 ; i++ )
{
CString file = dirfiles[ i ];
Trigger( file );
files.Add( file );
}
}
}
Thanks and Best Regards
Jugs
"A robust program is resistant to errors -- it either works correctly, or it does not work at all; whereas a fault tolerant program must actually recover from errors."
|
|
|
|
|