[filesystem] GetFileAttributesExA and Windows 95
Beman, you use the WinAPI call GetFileAttributesExA only twice in Boost.Filesystem, in _is_empty and file_size, but that means that the code won't run on Windows 95: file is linked to missing export KERNEL32.DLL:GetFileAttributesExA The msdn docs http://msdn.microsoft.com/library/en-us/fileio/fs/getfileattributesex.asp suggest that Win95 support requires only that you grab NewAPIs.h from the SDK Update Site. Here's an example of how it should be used: ========================================================================== http://codeproject.com/win32/harddiskspace.asp?df=100&forumid=15107&exp=0&select=466926 #define COMPILE_NEWAPIS_STUBS #define WANT_GETDISKFREESPACEEX_WRAPPER then in the file where you use GetDiskFreeSpaceEx.... #include <newapis.h> ========================================================================== Alternatively, I append a naïve implementation of the two files using GetFileAttributes and GetFileSize. Is any of this useful? Regards, Angus BOOST_FILESYSTEM_DECL bool _is_empty( const path & ph ) { DWORD const attributes = GetFileAttributes( ph.string().c_str() ); if ( attributes & INVALID_FILE_ATTRIBUTES ) boost::throw_exception( filesystem_error( "boost::filesystem::is_empty", ph, fs::detail::system_error_code() ) ); if ( attributes & FILE_ATTRIBUTE_DIRECTORY ) return is_empty_directory( ph ); DWORD file_size_high = DWORD const file_size_low = ::GetFileSize( ph.string().c_str(), &file_size_high ); if ( file_size_low == INVALID_FILE_SIZE ) { if ( GetLastError() = NO_ERROR ) boost::throw_exception( filesystem_error( "boost::filesystem::is_empty", ph, fs::detail::system_error_code() ) ); return ( !file_size_high && !file_size_low ); } BOOST_FILESYSTEM_DECL boost::intmax_t file_size( const path & ph ) { DWORD const attributes = GetFileAttributes( ph.string().c_str() ); if ( attributes & INVALID_FILE_ATTRIBUTES ) boost::throw_exception( filesystem_error( "boost::filesystem::is_empty", ph, fs::detail::system_error_code() ) ); if ( attributes & FILE_ATTRIBUTE_DIRECTORY ) boost::throw_exception( filesystem_error( "boost::filesystem::file_size", ph, "invalid: is a directory", is_directory_error ) ); DWORD file_size_high = DWORD const file_size_low = ::GetFileSize( ph.string().c_str(), &file_size_high ); if ( file_size_low == INVALID_FILE_SIZE ) { if ( GetLastError() = NO_ERROR ) boost::throw_exception( filesystem_error( "boost::filesystem::is_empty", ph, fs::detail::system_error_code() ) ); return (static_cast<boost::intmax_t>( file_size_high ) << (sizeof( file_size_low )*8)) + file_size_low }
Angus Leeming wrote:
Beman,
you use the WinAPI call GetFileAttributesExA only twice in Boost.Filesystem, in _is_empty and file_size, but that means that the code won't run on Windows 95:
file is linked to missing export KERNEL32.DLL:GetFileAttributesExA
The msdn docs http://msdn.microsoft.com/library/en-us/fileio/fs/getfileattributesex.asp suggest that Win95 support requires only that you grab NewAPIs.h from the SDK Update Site.
Actually, Beman, can I suggest the following patch? This allows the code to compile unchanged out of the box. If I want to support Win95 I need to define WANT_GETFILEATTRIBUTESEX_WRAPPER and provide NewAPIs.h. I can define the WANT_GETFILEATTRIBUTESEX_WRAPPER macro in my own config.h header file which is already #included by your library through BOOST_USER_CONFIG Thereafter, in some source file of my own I need only add #include <config.h> #define COMPILE_NEWAPIS_STUBS #include <NewAPIs.h> #undef COMPILE_NEWAPIS_STUBS and all works beautifully. Regards, Angus
"Angus Leeming" <angus.leeming@btopenworld.com> wrote in message news:dhbm6g$l6o$1@sea.gmane.org...
Angus Leeming wrote:
Beman,
you use the WinAPI call GetFileAttributesExA only twice in Boost.Filesystem, in _is_empty and file_size, but that means that the code won't run on Windows 95:
file is linked to missing export KERNEL32.DLL:GetFileAttributesExA
The msdn docs http://msdn.microsoft.com/library/en-us/fileio/fs/getfileattributesex.asp suggest that Win95 support requires only that you grab NewAPIs.h from the SDK Update Site.
Actually, Beman, can I suggest the following patch? This allows the code to compile unchanged out of the box. If I want to support Win95 I need to define WANT_GETFILEATTRIBUTESEX_WRAPPER and provide NewAPIs.h.
I can define the WANT_GETFILEATTRIBUTESEX_WRAPPER macro in my own config.h header file which is already #included by your library through BOOST_USER_CONFIG
Thereafter, in some source file of my own I need only add
#include <config.h>
#define COMPILE_NEWAPIS_STUBS #include <NewAPIs.h> #undef COMPILE_NEWAPIS_STUBS
and all works beautifully.
Angus, Have you tried the Boost.Filesystem "i18n" branch from CVS? In a week I'm due to present a proposal to the Library Working Group to add what is essentially Boost.Filesystem i18n to TR2. See http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1841.html Assuming the reaction is favorable, the plan is to move the i18n branch to the CVS HEAD. Thus while I'm quite interested in your suggestions, I'd like to see if they apply to the i18n code rather than the 1.33 code, which is essentially legacy code at this point. Thanks, --Beman
Beman Dawes wrote:
Angus, Have you tried the Boost.Filesystem "i18n" branch from CVS?
Nope, but I'll try and find some time to do so if you like. Trouble is, my Win95 tester is somewhere over the internet, so I'll have to rely on his good humour.
Assuming the reaction is favorable, the plan is to move the i18n branch to the CVS HEAD. Thus while I'm quite interested in your suggestions, I'd like to see if they apply to the i18n code rather than the 1.33 code, which is essentially legacy code at this point.
Right. But the fix might go in 1.33.x for some x, no? Here's another one for you: It seems that testing for a path "J:/foo/bar" on a Win98 or Win2000 machine (unsure which ATM) that doesn't have a J: drive is returning true falsely here because you ain't catching the correct error. Now, I have to say I don't agree with this policy of JM's, but suspect that the actual error is ERROR_INVALID_DRIVE. Will get back to you when my testers get back to me. BOOST_FILESYSTEM_DECL bool exists( const path & ph ) { if(::GetFileAttributesA( ph.string().c_str() ) == 0xFFFFFFFF) { UINT err = ::GetLastError(); if((err == ERROR_FILE_NOT_FOUND) || (err == ERROR_INVALID_PARAMETER) || (err == ERROR_PATH_NOT_FOUND) || (err == ERROR_INVALID_NAME)) // GetFileAttributes failed because the path does not exist return false; // for any other error we assume the file does exist and fall // through, this may not be the best policy though... (JM 20040330) return true; } return true; } Angus
It seems that testing for a path "J:/foo/bar" on a Win98 or Win2000 machine (unsure which ATM) that doesn't have a J: drive is returning true falsely here because you ain't catching the correct error. Now, I have to say I don't agree with this policy of JM's, but suspect that the actual error is ERROR_INVALID_DRIVE. Will get back to you when my testers get back to me.
I'm not sure I agree with that policy either: that's why there's a comment in the code that it may not be correct. The basic problem is we don't know which error codes represent "file does not exist", as opposed to some other error. The fact there is no documentation on which error codes that API can return makes the problem particularly tricky :-( John.
John Maddock wrote:
It seems that testing for a path "J:/foo/bar" on a Win98 or Win2000 machine (unsure which ATM) that doesn't have a J: drive is returning true falsely here because you ain't catching the correct error. Now, I have to say I don't agree with this policy of JM's, but suspect that the actual error is ERROR_INVALID_DRIVE. Will get back to you when my testers get back to me.
I'm not sure I agree with that policy either: that's why there's a comment in the code that it may not be correct. The basic problem is we don't know which error codes represent "file does not exist", as opposed to some other error. The fact there is no documentation on which error codes that API can return makes the problem particularly tricky :-(
I agree that it could potentially return any error code, but according to the Windows documentation, the correct way of dealing with errors is to say: if( !SomeAPIThatMayFail( ... )) { DWORD error = GetLastError(); // Process error based on values from winerror.h } winerror.h defines: ERROR_FILE_NOT_FOUND // "The system cannot find the file specified." ERROR_PATH_NOT_FOUND // "The system cannot find the path specified." ERROR_FILE_EXISTS // "The file exists." ERROR_POTENTIAL_FILE_FOUND // "A file was found, but it may not be the correct file." I think these are the main ones relating to file/path existence. Note that ERROR_FILE_EXISTS will be returned by things like CopyFile when attemping to copy to a file that already exists and you haven't specified to overwrite that file. - Reece
Reece Dunn wrote:
John Maddock wrote:
It seems that testing for a path "J:/foo/bar" on a Win98 or Win2000 machine (unsure which ATM) that doesn't have a J: drive is returning true falsely here because you ain't catching the correct error. Now, I have to say I don't agree with this policy of JM's, but suspect that the actual error is ERROR_INVALID_DRIVE. Will get back to you when my testers get back to me.
I'm not sure I agree with that policy either: that's why there's a comment in the code that it may not be correct. The basic problem is we don't know which error codes represent "file does not exist", as opposed to some other error. The fact there is no documentation on which error codes that API can return makes the problem particularly tricky
I agree that it could potentially return any error code, but according to the Windows documentation, the correct way of dealing with errors is to say:
if( !SomeAPIThatMayFail( ... )) { DWORD error = GetLastError(); // Process error based on values from winerror.h }
Sorry, John. That wasn't meant to be flaimbait :) Hi, Reece. I've little knowledge of the "right way" here, but the code in question does exactly what you suggest. It's just that it "plays God" a little and decides that only some errors are real errors. If the error isn't recognised then it tells the caller that all is well. Given that my code is of the form if (exists(path) && is_directory(path)) ... exists(path) returns true ATM causing is_directory(path) to throw_exception when it can't find path. Given that I'm not using exceptions in this code, I'd rather it lied conservatively rather than optimistically, if you get what I mean... The cure to my immediate problem is to have the actual error that my user is experiencing added to the list of known errors. My intuition tells me to expect ERROR_INVALID_DRIVE but I know we shouldn't write code based on intuition :) I'll post back the actual error as and when my bug reporter gets back to me with the results from a modified version of the code. (I got it to throw_exception instead of return true.) It strikes me that a useful addition to the code here would be to get the code to output a debug message indicating the (uncoded for but probably valid) error code. Regards, Angus
Sorry, John. That wasn't meant to be flaimbait :)
None taken.
Hi, Reece. I've little knowledge of the "right way" here, but the code in question does exactly what you suggest. It's just that it "plays God" a little and decides that only some errors are real errors. If the error isn't recognised then it tells the caller that all is well.
Given that my code is of the form
if (exists(path) && is_directory(path)) ...
exists(path) returns true ATM causing is_directory(path) to throw_exception when it can't find path. Given that I'm not using exceptions in this code, I'd rather it lied conservatively rather than optimistically, if you get what I mean...
Understood. The reason it's this way in the first place is that sometimes exists() was returning inconsistent values: in particular returning false when the file clearly did exist. However, depending on network traffic, which way the wind was blowing etc, the problem may or may not show up. Nice reliable OS this :-)
The cure to my immediate problem is to have the actual error that my user is experiencing added to the list of known errors. My intuition tells me to expect ERROR_INVALID_DRIVE but I know we shouldn't write code based on intuition :) I'll post back the actual error as and when my bug reporter gets back to me with the results from a modified version of the code. (I got it to throw_exception instead of return true.)
It strikes me that a useful addition to the code here would be to get the code to output a debug message indicating the (uncoded for but probably valid) error code.
Good idea, an OutputDebugString here would be a useful addition IMO. John.
John Maddock wrote:
The cure to my immediate problem is to have the actual error that my user is experiencing added to the list of known errors. My intuition tells me to expect ERROR_INVALID_DRIVE but I know we shouldn't write code based on intuition :) I'll post back the actual error as and when my bug reporter gets back to me with the results from a modified version of the code. (I got it to throw_exception instead of return true.)
Looks like my intuition was wrong: Exception caught: boost::filesystem::exists: "J:\MinSYS\home\Angus\lyx\devel\build-tex2lyx\LyX-1.4\Resources\locale": Access is denied. Assertion triggered in void boost::throw_exception(const std::exception&) by failing check "false" in file ../../../src/tex2lyx/boost.C:28 I guess that's the message associated with ERROR_ACCESS_DENIED, no? Having said that, I can't see the harm in testing for ERROR_INVALID_DRIVE too. This was on a machine running Win98SE. The machine has no J drive. Now I can imagine some people saying, hold on. Access denied doesn't mean that the file doesn't exist. However, I'd say that *as far as my program is concerned* that's exactly what it means... Hmmm. Sometimes the actual error is the worst possible news, no :( Any chance of this change going into the sources? Angus
Angus Leeming wrote:
John Maddock wrote:
The cure to my immediate problem is to have the actual error that my user is experiencing added to the list of known errors. My intuition tells me to expect ERROR_INVALID_DRIVE but I know we shouldn't write code based on intuition :) I'll post back the actual error as and when my bug reporter gets back to me with the results from a modified version of the code. (I got it to throw_exception instead of return true.)
Looks like my intuition was wrong:
Exception caught: boost::filesystem::exists: "J:\MinSYS\home\Angus\lyx\devel\build-tex2lyx\LyX-1.4\Resources\locale": Access is denied. Assertion triggered in void boost::throw_exception(const std::exception&) by failing check "false" in file ../../../src/tex2lyx/boost.C:28
I guess that's the message associated with ERROR_ACCESS_DENIED, no? Having said that, I can't see the harm in testing for ERROR_INVALID_DRIVE too.
"Access is denied." is the description for ERROR_ACCESS_DENIED, yes.
This was on a machine running Win98SE. The machine has no J drive.
Now I can imagine some people saying, hold on. Access denied doesn't mean that the file doesn't exist. However, I'd say that *as far as my program is concerned* that's exactly what it means...
Consider: if( !exists( "j:\\foo.txt" )) { std::ofstream out( "j:\\foo.txt" ); // oops! } if "j:\\foo.txt" exists, but you get an Access is denied error.
Hmmm. Sometimes the actual error is the worst possible news, no :(
Any chance of this change going into the sources?
The proper way to implement exists( ... ) on Windows is to do: #include "shlwapi.h" // link to shlwapi.lib bool exists( const path & ph ) { return ::PathFileExistsA( ph.string().c_str()) == TRUE; } MSDN: Function Information (PathFileExists) Minimum DLL Version shlwapi.dll version 4.71 or later Custom Implementation No Header shlwapi.h Import library shlwapi.lib Minimum operating systems Windows 2000, Windows NT 4.0 with Internet Explorer 4.0, Windows 98, Windows 95 with Internet Explorer 4.0 Unicode Implemented as ANSI and Unicode versions. or, as a fallback (for WinNT4/Win95 with IE3): bool exists( const path & ph ) { HANDLE file = CreateFileA( ph.string().c_str(), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL ); bool exist = file != INVALID_HANDLE_VALUE; ::CloseHandle( file ); return exist; } This should work for all cases. - Reece
"Reece Dunn" <msclrhd@hotmail.com> wrote in message news:dhf67p$5n7$1@sea.gmane.org...
... The proper way to implement exists( ... ) on Windows is to do:
#include "shlwapi.h" // link to shlwapi.lib
bool exists( const path & ph ) { return ::PathFileExistsA( ph.string().c_str()) == TRUE; }
In proposal to the standards committee, exists() is specified in terms of the results of POSIX stat(). On Windows, the best way to achieve that might be to use Windows Run-Time library's _stat() and _wstat() functions. The docs for those functions specifically say "A return value of -1 indicates an error, in which case errno is set to ENOENT, indicating that the filename or path could not be found." --Beman
Beman Dawes wrote:
Actually, Beman, can I suggest the following patch? This allows the code to compile unchanged out of the box. If I want to support Win95 I need to define WANT_GETFILEATTRIBUTESEX_WRAPPER and provide NewAPIs.h.
Angus, Have you tried the Boost.Filesystem "i18n" branch from CVS?
Hi, Beman. I still haven't tried out the i18n branch but my proposed patch has evolved. It will now compile against MinGW or MSVC out of the box and will enable Boost.Filesystem to run on Win95 boxes when the code is compiled against NewAPIs.h by defining only WANT_GETFILEATTRIBUTESEX_WRAPPER. It turned out that I had to jump through some preprocessing hoops in order to compile against MinGW's implementation of the winapi headers. I suggested that the MinGW developers make some changes to these headers to make them play well with NewAPIs.h but they have decided not to do so. They feel that the terms of the NewAPIs.h licence are too restrictive for them to read the source. Unfortunately, there is no publicly-available documentation of WANT_GETFILEATTRIBUTESEX_WRAPPER other than NewAPIs.h itself. Net result: no change :( That means, I'm afraid that gymnastics must be put into the Boost file. I attach the patch that I've proposed be added to the LyX source repository (Boost 1.32). It shows the changes to Boost.Filesystem and also the configure magic that I need to make it "just work". As you can see, I test for the existence of NewAPIs.h and then use this to define WANT_GETFILEATTRIBUTESEX_WRAPPER. This latter macro is all that Boost.Filesystem needs. Are you happy with this patch? If so, I'll try it out on the i18n branch and throw something the way of my Win95 tester to try out. If not... suggestions please! Regards, Angus ps, this is independent of the exists() discussion.
participants (4)
-
Angus Leeming -
Beman Dawes -
John Maddock -
Reece Dunn