OPEN on a folder name has different behavior in Win & Linux

[Bob the Hamster]

I just noticed something that seems to be a bug in FreeBasic 0.23

Code: Select all

DIM fh as integer = FREEFILE
DIM filename as string
filename = "folder_name"
IF OPEN(filename FOR BINARY AS #fh) THEN
 PRINT filename & " cannot be opened"
ELSE
 PRINT filename & " was opened okay, and is " & LOF(fh) & " bytes"
END IF
CLOSE #fh

In this example, folder_name is the name of a directory that actually exists in the current directory.

If you run this on Windows, the attempt to OPEN the directory will fail, as expected.
If you run this on Linux, the OPEN will succeed, and the LOF will be 4096 bytes.

EDIT: fixed the missing FREEFILE

[codeFoil]

Linux uses the UNIX "everything is a file" metaphor.

[Bob the Hamster]

That doesn't apply to this situation. Directory behavior should be consistent between Windows and Linux. A directory is not a file (not even in unix) so you shouldn't be able to open it like a file.

[TJF]

Directory behavior should be consistent between Windows and Linux. A directory is not a file (not even in unix) so you shouldn't be able to open it like a file.

Why? At least it's inconsistend due to different evaluation of letter cases.

Why shouldn't a directory get opened like a file? Use

Code: Select all

#INCLUDE "dir.bi"
IF LEN(DIR("folder_name", fbDirectory)) THEN ? "Directory!"

to check if it's a normal or a dirctory file.

BTW:

I don't think that

... on Linux, the OPEN will succeed, and the LOF will be 4096 bytes.

since your code uses zero as the file number (fh isn't initialized, the open statement should fail anyway).

Code: Select all

DIM fh as integer
DIM filename as string
filename = "folder_name"
IF OPEN(filename FOR BINARY AS #fh) THEN
...

[Bob the Hamster]

Oh! Thanks! I forgot the FREEFILE when I simplified the example code for pasting here. I should have just copied and pasted. I edited the first post to correct it.

I am just trying to report a bug here, I don't know why this is being interpreted as a debate about Linux filesystems ;)

[TJF]

I am just trying to report a bug here, I don't know why this is being interpreted as a debate about Linux filesystems ;)

This is no debate about LINUX file systems. And opening a directory as a file is no bug, it's a feature! Why should FB limit LINUX features when they aren't available on win?

Oh! Thanks! I forgot the FREEFILE when I simplified the example code for pasting here. I should have just copied and pasted. I edited the first post to correct it.

When you're interested in a good example then add a CLOSE #fh statement as well :)

[Bob the Hamster]

It just seems to me like OPEN should be consistent. If it was up to me (and I under stant it ISN'T up to me) I would make OPEN behave the same way on all platforms, and then make a separate command like RAWOPEN or UNIXOPEN or something like that. Then the normal use case would be consistent, and the obscure use case would still be possible.

But it doesn't matter much. The issue is easy to workaround now that I know about it.

It would be nice if the documentation page at http://www.freebasic.net/wiki/wikka.php?wakka=KeyPgOpen mentioned what behavior to expect if you try to open a folder... I would edit it myself, but I can't seem to log into the wiki for some reason :P

EDIT: nevermind, It was just a cookie problem, I will document it.

[TJF]

It just seems to me like OPEN should be consistent. If it was up to me (and I under stant it ISN'T up to me) I would make OPEN behave the same way on all platforms, and then make a separate command like RAWOPEN or UNIXOPEN or something like that. Then the normal use case would be consistent, and the obscure use case would still be possible.

It's up to you. You are free to code that bevaviuor by using a preprocessor macro. But it will slow down the execution speed and that's why I think it shouldn't be in the fbc.

EDIT: nevermind, It was just a cookie problem, I will document it.

Good idea!

[counting_pine]

Thanks Bob. It should go under Platform Differences.
So what happens when you open a folder a file? What are the contents? Is it read-only?

[Bob the Hamster]

I just did some more testing. The fole hande cannot be written to or read from. (if you tried to read you just get zeroes)

I am having trouble figuring out why the c library on unix allows this at all. At first I though maybe it was just to get the os file descriptor for a later call to stat(), but that doesn't make sense because stat has an overload to open by filename.

It is a mystery to me.

I found a bug report of this from back in 2007 http://sourceforge.net/tracker/index.ph ... tid=693196 but v1ctor decided not to fix it citing performance concerns, but I was talking this over with teeemcee (who knos more about this stuff than I do) and he said:

I'm quite shocked that fopen()ing a directory works on linux, since
there's no use for an fopen()ed directory aside from getting the
underlying file descriptor and using that.

I don't buy the argument that checking for a directory adds
significant time; it would certainly be dwarfed by the open itself.
I'm not going to bother timing it, but I assume that all the data for
an fstat() call would be found in the kernel's inode cache; so I
estimate ~1 microsecond.

I am willing to try an write a patch for this, if it is desired.

EDIT: and If I could do that, I guess it would be pretty easy to profile and see if there really is a speed difference or not

[counting_pine]

Does it produce error codes if you try to Get/Put with the file handle, or does it just "succeed"?

It wouldn't be entirely down to me, but I'd vote to accept a "clean" patch if there's not much overhead. By that I mean, if it can be tested with a function of the same kind as fopen, or if a test can be performed on the opened handle, then that would be OK.

I'm not an expert: I could probably fudge together something "unclean" with dir() or chdir() or something like that. But I'd worry that such solutions could result in edge case incompatibilities or side-effects.

(This reminds me a little of the case that surfaced a while ago where DOS could delete read-only files. I don't think we settled on an adequate solution to that one.)

By the way, I moved the warning. I also decided to remove the Windows bit: I can't test for DOS, but at the same time I want to give the impression that Linux is the exception.

[TeeEmCee]

Of course on second thoughts it's not surprising that fopen() allows opening directories: library implementors also wanted to avoid unnecessary overhead. Linux's open() accepts a flag to open JUST directories, but none to specify JUST files.

I would make OPEN behave the same way on all platforms, and then make a separate command like RAWOPEN or UNIXOPEN or something like that.

Already exists: use open() from FB's Unix headers. Using FB file handles on top of stdio FILEs on top of OS file descriptors seems like a bad idea. (I personally wouldn't touch the C->FB translated unix headers though; they're non-portable and full of errors and omissions)

Does it produce error codes if you try to Get/Put with the file handle, or does it just "succeed"?

The underlying fread/fwrite calls will fail, so surely the FB functions forward the errors.

It wouldn't be entirely down to me, but I'd vote to accept a "clean" patch if there's not much overhead. By that I mean, if it can be tested with a function of the same kind as fopen, or if a test can be performed on the opened handle, then that would be OK.

Yes, you would use fstat(), which is faster than stat(). However that's of course platform specific, while the OPEN implementation in libfb_dev_file_open.c is in shared code. So it seems that to add this check you would have to add a hook to a platform-specific check elsewhere.

[Bob the Hamster]

Does it produce error codes if you try to Get/Put with the file handle, or does it just "succeed"?

The underlying fread/fwrite calls will fail, so surely the FB functions forward the errors.

Actually, GET on a directory's file handles reads zeroes, and GET as a function returns 0, so the read is succeeding.

It wouldn't be entirely down to me, but I'd vote to accept a "clean" patch if there's not much overhead. By that I mean, if it can be tested with a function of the same kind as fopen, or if a test can be performed on the opened handle, then that would be OK.

Yes, you would use fstat(), which is faster than stat(). However that's of course platform specific, while the OPEN implementation in libfb_dev_file_open.c is in shared code. So it seems that to add this check you would have to add a hook to a platform-specific check elsewhere.

Yes, exactly. Last weekend I was easily able to write a simple patch for this, but it was unix-only and I don't think it would compile on Windows.

It seems like would be pretty trivial to wrap the code in an #ifdef but I don't see any other examples of places where that is done for unix-only code, so i don't want to be the first to do it.

Also, my patch used stat(fname, &info) instead of fstat(fp, &info) because when I used fstat() I got warnings like this:

Code: Select all

rtlib/dev_file_open.c:173:5: warning: passing argument 1 of ‘fstat’ makes integer from pointer without a cast [enabled by default]

I understand why stat() could be slightly slower than fstat() but when I tested my code by rapidly opening and closing 10000 files, the difference between stat() and no check at all was too small for me to notice.

[dkl]

Changing Open behaviour to fail to open directories is fine with me, since opening directories is not useful anyways.

It may be necessary to check all fopen()'s in the rtlib though, for example besides fb_DevFileOpen() it may also be necessary to adjust fb_DevFileOpenEncod(). And then, the vbcompat.bi functions FileLen() and FileCopy() should probably also fail on directories (in case they do not yet).

Search the source tree for HOST_UNIX, and you'll find there are plenty if #ifdefs checking for it, so adding some more is not an issue.

With this in mind: http://stackoverflow.com/questions/9709 ... -from-poin

I think something like this would work in fb_DevFileOpen():

Code: Select all

#if defined( HOST_UNIX )
	struct stat info;
	if( fstat( fileno( fp ), &info ) == 0 ) {
		if( S_ISDIR( info.st_mode ) ) {
			/* Refuse to OPEN a directory */
			fclose( fp );
			FB_UNLOCK( );
			return fb_ErrorSetNum( FB_RTERROR_FILENOTFOUND );
		}
	}
#endif