Version: unspecified OS: Linux as per https://bugs.kde.org/show_bug.cgi?id=269300 TagLib opens files O_RDWR unnecessarily. This is potentially dangerous -- in the case of a bug in the application using TagLib -- and it also violates the principle of least privilege. In addition to that, opening the songs O_RDWR prevents Amarok from using the fscache infrastructure for NFS and CIFS mounts. I suggest TagLib open the file only O_READONLY and when it needs to write data to the file, reopen it O_RDWR. If a permission check needs to be done to find out if the file is writable -- for user interface reasons, or what not -- then access() at the appropriate time should work equally well. Alternatively, I suspect that opening it O_RDWR, then UNCONDITIONALLY reopening it O_READONLY until it's time to modify the file, could work too. Reproducible: Always
I have no problem with a read-only mode for the TagLib API (i.e. save() would always fail), but I'm not willing to implement a mixed mode with file reopening. If an application expects to save the file, it should open the file for writing at the beginning. Regarding access(), it doesn't work equally well, see https://github.com/taglib/taglib/commit/320b76d95 for details.
I'd be content with a readonly mode, except applications would have to code in extra efforts for that. So why not do it for them by default? It's just a freopen or something similar, whenever apps call a method that requires saving to disk, and then the API would not change at all. It's actually simpler, I believe, to implement that.
Yes, it would be easier to implement, but I just don't like the idea or reopening the file.
Well, why not? I'm really curious to know why, especially if it's admittedly easier to implement.
File opening is an operation that is very likely to fail. I personally prefer to have an explicit point where it can fail. Having it done automatically means that applications do not necessarily know at which point can it fail and how to handle it. I'd rather have a method that reopens the file in write mode explicitly, or ideally to decide it in the constructor.
But you would not be opening the file, merely the file descriptor of a file already opened, an you would already have tested it the file was openable readwrite in the constructor too... I don't see how that objection holds.
Well, I don't see how switch the from readonly to readwrite mode and not have to open it again.
It would also break existing application logic since the assumption is that the value of File::readOnly() can be read immediately after opening. The only alternative that I see would be to try to open read/write, set a flag, close, reopen read only, then later reopen again read/write, which is a bit bleh (and could still fail since the permissions could change in the meantime).
That is what I suggested. Permissions could have changed but the current model has the same problem (at least on oses with the revoke call).
freopen if you are using FILEs. close and open if you are using fds. El Friday, March 25, 2011, Lukáš Lalinský escribió: > https://bugs.kde.org/show_bug.cgi?id=269395 > > > > > > --- Comment #7 from Lukáš Lalinský <lalinsky gmail com> 2011-03-25 > 21:56:19 --- Well, I don't see how switch the from readonly to readwrite > mode and not have to open it again.
*** Bug 269300 has been marked as a duplicate of this bug. ***
*** This bug has been marked as a duplicate of bug 167329 ***