> the author entered them backwards You're mistaken. The author originally suspected - or hypothesized - that was possibly the bug, but the actual cause detai

> the author entered them backwards You're mistaken. The author originally suspected - or hypothesized - that was possibly the bug, but the actual cause detailed in the post ended up being different (nondeterministic hashmap iteration over components) and the final patch does not include the initially proposed "fix" of swapping the red and blue components as was shown towards the very top of the article. The original order was correct. Swapping red/blue would've just been canceling out one bug by adding another. https://github.com/SerenityOS/serenity/commit/a10ad24c760bfe713f1493e49dff7da16d14bf39 https://github.com/SerenityOS/serenity/commit/a10ad24c760bfe... > The type system can prevent this flaw. The type system can add some roadbumps to encourage centralizing the conversion of "untyped" disk bytes to typed r/g/b components or a typed color component, but that conversion must still occur somewhere, and it can be written backwards at that somewhere, and one of those somewheres will be inside JPGLoader.cpp. That's the fundamental job of a JPEG loader - conversion of an untyped byte stream into useful types for the rest of the program to use. Additional use of the type system might limit where within JPGLoader.cpp such a component swapping flaw might be likely, but it's never going to prevent it for all of JPGLoader.cpp. The existence of the Color type already makes such a flaw fairly unlikely outside of JPGLoader.cpp and similar conversion points. That's sufficient use of the type system. More within JPGLoader.cpp, while possible, would be overkill based on my own experience with color conversion and swapped component bugs. Hell, to play devil's adovcate, further use of the type system could cause such flaws! The extra code is likely to cause additional reviewer fatigue in code reviews. Reviewer fatigue causes inattention. Inattention causes the reviewer to miss swapped or nondeterministics components. Perhaps such inattention caused this bug! I would be in favor of replacing: const Color color { (u8)block.y[pixel_index], (u8)block.cb[pixel_index], (u8)block.cr[pixel_index] }; with: const auto color = Color::from_rgb((u8)block.r[pixel_index], (u8)block.g[pixel_index], (u8)block.b[pixel_index]); To make it clearer that this code is correct as is. Doesn't add any new types though, just names a constructor in such a way as to imply an argument order, and uses the existing r/g/b union aliases of y/cb/cr in macroblocks (r/g/b is more correct at that point in the code, since ycbcr_to_rgb should've been called already - it's possible this code was written before those union aliases were added?) It's still possible to have a copy+paste bug with my replacement by using r/g/g as input instead of r/g/b or similar, but I've seen such bugs even with per-component types - just need the first copy to be mistaken and copy+pasted all over.