On Mon, Aug 08, 2022 at 04:32:39PM -0700, Joanne Koong wrote: [...] > > It being a read-only const is was why I made this a BUILD_BUG_ON. My > > intention here was to ensure that we're not accidentally skipping the > > resetting of caller_saved[0]. The original code iterated from > > caller_saved[0] -> caller_saved[CALLER_SAVED_REGS - 1]. Now that we're > > starting from caller_saved[1], this compile-time assertion verifies that > > we're not accidentally skipping caller_saved[0] by checking that it's the > > same as BPF_REG_0, which is reset above. Does that make sense? > > I think it's an invariant that r0 - r5 are the caller saved args and > that caller_saved[0] will always be BPF_REG_0. I'm having a hard time > seeing a case where this would change in the future, but then again, I > am also not a fortune teller so maybe I am wrong here :) I don't think > it's a big deal though so I don't feel strongly about this I agree that it seems very unlikely to change, but I don't see the harm in leaving it in. Compile time checks are very fast, and are meant for cases such as these to check constant, build-time invariants. If you feel strongly, I can remove it. Thanks, David
