Problem with FileExists() function
Mark Jacobs
example, FileExists returns false for the file. It returns true for
normally-dated ones, like 9/3/2007. I wrote a replacement function for it
which works for all dates :-
bool mjflexist(AnsiString fnm)
{
struct stat statbuf;
return (stat(fnm.c_str(),&statbuf)==0);
}
--
Mark Jacobs
http://jacobsm.com
Remy Lebeau (TeamB)
"Mark Jacobs" <www.jacobsm.com/mjmsg.htm?Borland Newsgroup> wrote in
message news:45f1...@newsgroups.borland.com...
> If I make a file have a last modified date under NTFS of 9/3/3011,
> for example, FileExists returns false for the file. It returns true
> for normally-dated ones, like 9/3/2007.
FileExists() is implemented completely wrong to begin with. It relies
on FileAge(), which checks the file's DOS timestamp. Such timestamps
can't handle dates above 12/31/2107. FileExists() should not care
about the file's timestamp at all. That is a known issue that has
already been reported to Borland awhile ago but has not been fixed
yet.
> I wrote a replacement function for it which works for all dates
You could alternatively use GetFileAttributes(), like
DirectoryExists() does, ie:
bool mjflexist(const AnsiString &fnm)
{
DWORD dwAttrs = ::GetFileAttributes(fnm.c_str());
return ((dwAttrs != 0xFFFFFFFF) && ((dwAttrs &
FILE_ATTRIBUTE_DIRECTORY) == 0));
}
> return (stat(fnm.c_str(),&statbuf)==0);
You are not verifying whether the returned stats are actually for a
file and not a directory. If you look at the implementation of
DirectoryExists() for Linux, it does use stat() the same way you do,
except that it checks the st_mode value for the __S_IFDIR flag, so you
should be doing the same, ie:
bool mjflexist(const AnsiString &fnm)
{
struct stat statbuf;
if( stat(fnm.c_str(), &statbuf) == 0 )
return ((statbuf.st_mode & __S_IFDIR) == 0);
else
return false;
}
Gambit
David Dean [CodeGear]
"Remy Lebeau \(TeamB\)" <no....@no.spam.com> wrote:
> FileExists() should not care
> about the file's timestamp at all. That is a known issue that has
> already been reported to Borland awhile ago but has not been fixed
> yet.
Do you happen to recall the QC/RAID number?
--
-David Dean
CodeGear C++ QA Engineer
<http://blogs.codegear.com/ddean/>
Mark Jacobs
news:45f1e442$1...@newsgroups.borland.com...
> You could alternatively use GetFileAttributes(), like
> DirectoryExists() does, ie:
>
> bool mjflexist(const AnsiString &fnm)
> {
> DWORD dwAttrs = ::GetFileAttributes(fnm.c_str());
> return ((dwAttrs != 0xFFFFFFFF) && ((dwAttrs &
> FILE_ATTRIBUTE_DIRECTORY) == 0));
> }
>
>> return (stat(fnm.c_str(),&statbuf)==0);
>
> You are not verifying whether the returned stats are actually for a
> file and not a directory. If you look at the implementation of
> DirectoryExists() for Linux, it does use stat() the same way you do,
> except that it checks the st_mode value for the __S_IFDIR flag, so you
> should be doing the same, ie:
>
> bool mjflexist(const AnsiString &fnm)
> {
> struct stat statbuf;
> if( stat(fnm.c_str(), &statbuf) == 0 )
> return ((statbuf.st_mode & __S_IFDIR) == 0);
> else
> return false;
> }
Thanks for those important suggestions. I note the use of & for the
AnsiString fnm argument in both cases. Is this more efficient than simply
passing the string itself? Does the & avert the need to "copy" the string
onto the stack?
Remy Lebeau (TeamB)
> Thanks for those important suggestions. I note the use of & for the
> AnsiString fnm argument in both cases. Is this more efficient than
> simply passing the string itself?
Yes. Even though the character data is reference-counted internally
to avoid unnecessary allocations, passing an AnsiString by value
causes a temporary AnsiString object to be constructed and then the
character block copied to it. If you use a reference instead, then
the original AnsiString can be used as-is without any copying.
Also, if you pass an AnsiString by non-const reference, then you will
get a compiler warning if you try to pass in a char* to the parameter,
ie:
bool mjflexist(AnsiString &fnm);
if( mjflexist("the path here") )
...
[C++ Warning] Unit1.cpp(18): W8030 Temporary used for parameter
'fnm' in call to 'mjflexist(AnsiString &)'
You would have to use an explicit construction to get around it, ie:
if( mjflexist(AnsiString("the path here")) )
...
If you pass the AnsiString by const reference instead, then the
compiler can do an implicit construction for you without issuing a
warning, ie:
bool mjflexist(const AnsiString &fnm);
if( mjflexist("the path here") )
...
Gambit
Remy Lebeau (TeamB)
"David Dean [CodeGear]" <david....@spam.codegear.com> wrote in
message news:david.dean.no-B58...@killface.local...
> Do you happen to recall the QC/RAID number?
QC #3513, RAID #195136
Looking at the report, the issue appears to already have been fixed in
D2005, though. I don't have D2005, but the implementation in BDS 2006
is using GetFileAttributes() now as I described earlier. However, it
is using Integer instead of DWORD for the attributes. See QC #20590,
RAID #233968 about that issue.
Gambit
David Dean [CodeGear]
"Remy Lebeau \(TeamB\)" <no....@no.spam.com> wrote:
> However, it
> is using Integer instead of DWORD for the attributes. See QC #20590,
> RAID #233968 about that issue.
Thanks Remy. It looks like R&D is already investigating this. I'll
pester them to try and raise the visibility of this issue.
Hans Galema
> If I make a file have a last modified date under NTFS of 9/3/3011, for
> example, FileExists returns false for the file.
Very good.
Files that pretend to be created in the future should be neglected.
Don't blame FileExists but the functions that maka it possible to
to imprint a date in the future.
Hans.
Remy Lebeau (TeamB)
"Hans Galema" <not...@notused.nl> wrote in message
news:4664...@newsgroups.borland.com...
> Don't blame FileExists
FileExists() is known to be wrongly implemented in the first place.
It can fail on other times, not just future times. Borland uses the
file's timestamps to determine if the file exists, which is wasted
overhead. FileExists() should be implemented the same way
DirectoryExists() is - to simply call the GetFileAttributes() API
function and nothing else.
Gambit
Paul at NCF
Do you have other examples of how this fails, for future reference? As I
believe I'm using this funtion (like many other people I suspect).
Regards,
Paul
"Remy Lebeau (TeamB)" <no....@no.spam.com> wrote in message
news:4664...@newsgroups.borland.com...
>
> "Hans Galema" <not...@notused.nl> wrote in message
> news:4664...@newsgroups.borland.com...
>
Remy Lebeau (TeamB)
"Paul at NCF" <notpl...@clara.co.uk> wrote in message
news:4665...@newsgroups.borland.com...
> Do you have other examples of how this fails, for future reference?
This topic has been discussed many times before. Please go to
http://www.deja.com and search through the newsgroup archives.
Gambit
Ed Mulroy
function of your own. Something like this for example:
-----------------------------
#include <windows.h>
const DWORD NOT_FOUND = 0xFFFFFFFF;
const DWORD DONT_WANT =
FILE_ATTRIBUTE_DIRECTORY |
FILE_ATTRIBUTE_OFFLINE;
bool WINAPI ExistsFile(const char *file_name)
{
DWORD attr = GetFileAttributes(file_name);
return (attr != NOT_FOUND) && !(attr & DONT_WANT);
}
#ifdef INC_VCL
bool WINAPI ExistsFile(const AnsiString & as)
{
return ExistsFile(as.c_str());
}
#endif
-----------------------------
. Ed
> Paul at NCF wrote in message
> news:4665...@newsgroups.borland.com...
David Dean [CodeGear]
"Remy Lebeau \(TeamB\)" <no....@no.spam.com> wrote:
> FileExists() should be implemented the same way
> DirectoryExists() is - to simply call the GetFileAttributes() API
> function and nothing else.
This should have been fixed in BDS 2006.
Bruce Salzman
> In article <4664...@newsgroups.borland.com>,
> "Remy Lebeau \(TeamB\)" <no....@no.spam.com> wrote:
>
>> FileExists() should be implemented the same way
>> DirectoryExists() is - to simply call the GetFileAttributes() API
>> function and nothing else.
>
> This should have been fixed in BDS 2006.
>
It was.
--
Bruce