4 ms·
> Changing the behaviour of a file mode from truncate to not-truncate is a questionable decision Looking at the commit which changed this behavior: https://cs.
by progval 4y ago
> Changing the behaviour of a file mode from truncate to not-truncate is a questionable decision
Looking at the commit which changed this behavior: https://cs.android.com/android/_/android/platform/frameworks/base/+/63280e06fc64672ab36d14f852b13df2274cc328 https://cs.android.com/android/_/android/platform/frameworks...
it seems like it was a refactoring gone wrong, and not a conscious decision to change the behaviour.
- ryanjshaw 4y agoNice find. You may be correct that it wasn't intentional due to the lack of documentation around the change, but I do note they updated tests for the refactored function [1], and the tests clearly show different behaviour from the original implementation [2]. I'm surprised this wasn't picked up during code review. Also surprised no unit tests for the original implementation, just the new one - if you're going to refactor something, add tests for the original implementation if there aren't any already. [1] https://cs.android.com/android/_/android/platform/frameworks/base/+/63280e06fc64672ab36d14f852b13df2274cc328:core/tests/coretests/src/android/os/FileUtilsTest.java;dlc=7bd671e11fcfe956b78087c9ac27f25c1dee6e3e https://cs.android.com/android/_/android/platform/frameworks... [2] https://cs.android.com/android/_/android/platform/frameworks/base/+/63280e06fc64672ab36d14f852b13df2274cc328:core/java/android/os/ParcelFileDescriptor.java;dlc=7bd671e11fcfe956b78087c9ac27f25c1dee6e3e https://cs.android.com/android/_/android/platform/frameworks...
- hgsgm 4y agoWho gets promoted faster: The person who spends an extra hour or two on every code review investigating the behavior of hacky wrappers 50-year-old awkward file APIs, looking for the one and 100 critical security bug? Or The person who gets more features launched?