IsWritable / IsReadable methods do not currently check Write/Read mode bits.

[ver]

In BeBook, it says IsReadable and IsWritable methods on BFile: "Note that these functions don't query the actual file to check permissions, they only tell you what the access request was when the BFile object was initialized." which makes me ask.. "Why not?"

The developer is aware of the flag they used when they instantiate a BFile, so these methods seem redundant.

An application (Pe, for example,) must fstat the file (GetStat()), and then sift through mode flags to decipher if the read or write bits are set, which is several lines of code which would likely require a lot of reimplementing.

This patch implements mode checking in IsWritable and IsReadable.

Please note that the file is statted once for each time either of these methods are called. This is done because the mode of the file may change AFTER opening. (Eg, a read-only flagged text file is opened in StyledEdit, the user attempts to save, a warning dialogue appears to inform the user of the read-only setting of the file. The user cancels, updates the mode of the file, and selects Save again. If the stat is done only when the file is initially opened, the mode continue to indicate a read-only status.)

[ver]

Revision to stat only if InitCheck() results in B_OK

[axeld]

These changes make no sense - if a file is not writable for you, you cannot open it read/write anyway, hence the additional check is superfluous.

Furthermore, we cannot change original Be API, since we aim for binary compatibility (ie. if we would actually change such a method, we'd need to use symbol versioning in order to make it still work as expected for legacy applications).

[axeld]

(In #5521) Besides that #5520 has been marked as invalid, your patch also contains a number of style violations - we do have a coding style and require all of our own sources to comply to it.

Anyway, StyledEdit obviously issues no warning, as writing actually succeeds (since you are the root user), so the file is writable after all. I guess it would make sense to check for the writable bit being set; so while this patch isn't adequate, I'm keeping the bug open.

BTW: Instead of fstat(), there is also a BFile::GetStat() inherited from BStatable.

[ver]

Replying to axeld:

These changes make no sense - if a file is not writable for you, you cannot open it read/write anyway, hence the additional check is superfluous.

Why would you set a file mode to r-x if you never check the w? What is the point of IsWritable() otherwise? To tell me I passed THIS flag when I called my BFile constructor? :/

Furthermore, we cannot change original Be API, since we aim for binary compatibility (ie. if we would actually change such a method, we'd need to use symbol versioning in order to make it still work as expected for legacy applications).

Meh, alright, I wasn't aware it would break binary compatibility. So, would the solution be to effectively move this check directly into StyledEdit code, in that case?

[anevilyak]

Replying to ver:

Why would you set a file mode to r-x if you never check the w? What is the point of IsWritable() otherwise? To tell me I passed THIS flag when I called my BFile constructor? :/

How does a different function call in another place in the program that may have to deal with many different BFile pointers know which flags a given one was opened with? It doesn't, hence those accessors.

[axeld]

Meh, alright, I wasn't aware it would break binary compatibility.

It breaks compatibility on a functional level - if an application is using IsWritable(), it will not work as expected all of a sudden anymore.

So, would the solution be to effectively move this check directly into StyledEdit code, in that case?

Yes, indeed, I'm afraid that would be the solution.

[stippi]

ver, what you seem to be missing, and what the other commenters write between the lines only (unless I overlooked it), is that the open mode of a BFile object cannot change. You create a new BFile object, or you call SetTo() on an existing BFile object. In both cases, you have to provide the open mode. What you seem to be missing is that this operation will simply fail if the desired open mode is not allowed. You cannot create BFile with B_READ_FILE | B_WRITE_FILE, have it succeed and in reality, the file is only readable. Instead, SetTo() or InitCheck() will return an error code, and you just cannot use the object. That's why there is no problem with IsWritable() and IsReadable() at all, since the mode cannot change after construction or SetTo(). The usecase you pointed out with the user changing permissions of the file makes no sense, since StyledEdit for example would have to reopen the file internally, for any changes to take affect, it won't have an affect on already open BFile objects.

[axeld]

There is a point for this for root users, though, since they can just write to write protected files. Normally, editors issue a warning for this, but this has to be specifically coded everywhere.

A handy getter wouldn't be too bad, but that should probably be end up in BStatable, and should get a better fitting name.

[bonefish]

Replying to axeld:

There is a point for this for root users, though, since they can just write to write protected files. Normally, editors issue a warning for this, but this has to be specifically coded everywhere.

A handy getter wouldn't be too bad, but that should probably be end up in BStatable, and should get a better fitting name.

Yeah, we should call it GetPermissions(). Oh wait, that already exists. ;-)

[axeld]

I know about GetPermissions() - I meant a function that also takes into account your group info, for example.