5 ms·
Mentioned in the link. This seems to be the original PR for the API in question, ParcelFileDescriptor#parseMode(): https://android.googlesource.com/platform/f
by canucker2016 4y ago
Mentioned in the link.
This seems to be the original PR for the API in question, ParcelFileDescriptor#parseMode():
https://android.googlesource.com/platform/frameworks/base/+/eb8c3f93edc826413ff4143284dec01c1061d5cc%5E%21/#F3 https://android.googlesource.com/platform/frameworks/base/+/...
+ public static int parseMode(String mode) {
+ final int modeBits;
+ if ("r".equals(mode)) {
+ modeBits = ParcelFileDescriptor.MODE_READ_ONLY;
+ } else if ("w".equals(mode) || "wt".equals(mode)) {
+ modeBits = ParcelFileDescriptor.MODE_WRITE_ONLY
+ | ParcelFileDescriptor.MODE_CREATE
+ | ParcelFileDescriptor.MODE_TRUNCATE;
+ } else if ("wa".equals(mode)) {
...
You can see that "w" and "wt" map to the same mode for file operations, including the MODE_TRUNCATE flag.
But we jump ahead a few years and the code is reworked.
from https://android.googlesource.com/platform/frameworks/base/+/63280e06fc64672ab36d14f852b13df2274cc328%5E%21/core/java/android/os/ParcelFileDescriptor.java https://android.googlesource.com/platform/frameworks/base/+/...:
public static int parseMode(String mode) {
- final int modeBits;
- if ("r".equals(mode)) {
- modeBits = ParcelFileDescriptor.MODE_READ_ONLY;
- } else if ("w".equals(mode) || "wt".equals(mode)) {
- modeBits = ParcelFileDescriptor.MODE_WRITE_ONLY
- | ParcelFileDescriptor.MODE_CREATE
- | ParcelFileDescriptor.MODE_TRUNCATE;
- } else if ("wa".equals(mode)) {
...
- return modeBits;
+ return FileUtils.translateModePosixToPfd(FileUtils.translateModeStringToPosix(mode));
}
So what does FileUtils.translateModePosixToPfd(FileUtils.translateModeStringToPosix(mode)) do?
from https://android.googlesource.com/platform/frameworks/base/+/63280e06fc64672ab36d14f852b13df2274cc328%5E%21/#F2 https://android.googlesource.com/platform/frameworks/base/+/...:
+ public static int translateModeStringToPosix(String mode) {
+ int res = 0;
+ if (mode.startsWith("rw")) {
+ res |= O_RDWR | O_CREAT;
+ } else if (mode.startsWith("w")) {
+ res |= O_WRONLY | O_CREAT;
+ } else if (mode.startsWith("r")) {
+ res |= O_RDONLY;
+ } else {
+ throw new IllegalArgumentException("Bad mode: " + mode);
+ }
+ if (mode.indexOf('t') != -1) {
+ res |= O_TRUNC;
+ }
+ if (mode.indexOf('a') != -1) {
+ res |= O_APPEND;
+ }
+ return res;
+ }
Now "w" and "wt" map to different Posix open mode flags since one has to explicitly pass in "t" to get file truncation.
A dev was careless, plain and simple.
Having code in a monorepo doesn't help if you don't check how your code changes affects consumers/clients of the code.
If they couldn't change translateModeStringToPosix() behaviour, then the dev could've amended parseMode() to map "w' to "wt" to keep the old behaviour (ugly and non-orthogonal, yes, but that's the way the API was designed).
- canucker2016 4y agoLooking at the reworked parseMode API some more and it looks like a total mess. Original code would only accept one of these modes: "r", "w", "wt", "wa", "rw", "rwt". Anything else, and you'll get an IllegalArgumentException exception. which matches the docs: from https://developer.android.com/reference/android/os/ParcelFileDescriptor#parseMode(java.lang.String) https://developer.android.com/reference/android/os/ParcelFil... Parameters mode String: The string representation of the file mode. Can be "r", "w", "wt", "wa", "rw" or "rwt". Throws IllegalArgumentException if the given string does not match a known file mode. ---- The modified parseMode code will only throw the IllegalArgumentException if the mode string does NOT start with "rw", "w", or "r". You can append any other character(s) to the mode string ("a", "t", "x", "y", "z", your fav emoji char), and you won't get an IllegalArgumentException exception now. I wonder what happens when open() gets a mode flag with both "a" and "t" set? parseMode API contract thrown out the window...
- masklinn 4y ago> I wonder what happens when open() gets a mode flag with both "a" and "t" set? Nothing I expect. O_TRUNC and O_APPEND are not incompatible, O_APPEND is just redundant (because by definition if you truncate the file you're writing at its end). macOS certainly doesn't seem to care when I do it.
- cesarb 4y ago> O_APPEND is just redundant (because by definition if you truncate the file you're writing at its end). No, O_APPEND is not redundant with O_TRUNC. When you use O_APPEND, you also have the extra behavior that every write goes to the end of the file, instead of the current position; this is important when more than one process is writing to the same file.
- deleted 4y ago[deleted]
- makomk 4y agoThe original C/POSIX API this is based on is non-orthogonal because the combinations it doesn't support generally don't make sense. There's usually no point opening files for reading or in append mode if you're truncating the existing contents anyway, and not truncating when opening in write-only mode is almost always wrong because any existing contents that aren't overwritten are almost certainly not relevant to the new contents and shouldn't be there (because the app clearly isn't payint attention to what was already there).