On Tue, Sep 16, 2025 at 05:59:18AM -0400, Joel Fernandes wrote: [...]> > > In C also this is valid. If you passed a higher value than what the > > > bitfield can hold, the compiler will still just use the bits that it > > > needs and ignore the rest. > > > > In C we've got FIELD_{PREP,GET,MODIFY}, implementing the checks. > > So those who want to stay on safe side have a choice. > > Ah ok. We can add these checks then for the accessors, I will do so in v4.The C checks use BUILD_BUG_ON, in rust-for-linux we have build_assert but it is fragile and depends on the value being a constant. Since the setter API accepts a run-time value and not a constant, we cannot use this. Or, we can fail at runtime, but that requires changing the set_* to try_set_* and returning a Result instead of Self. Alternatively, we can have a debug option that panics if the setter API is misued. Thoughts? Or for the moment, we can keep it simple and filter out / ignore extra bits of the larger value passed (which is what nova-core's register macro bitfield implementation currently does anyway). thanks, - Joel
On 9/19/25 5:39 PM, Joel Fernandes wrote:> On Tue, Sep 16, 2025 at 05:59:18AM -0400, Joel Fernandes wrote: > [...] >>>> In C also this is valid. If you passed a higher value than what the >>>> bitfield can hold, the compiler will still just use the bits that it >>>> needs and ignore the rest. >>> >>> In C we've got FIELD_{PREP,GET,MODIFY}, implementing the checks. >>> So those who want to stay on safe side have a choice. >> >> Ah ok. We can add these checks then for the accessors, I will do so in v4. > > The C checks use BUILD_BUG_ON, in rust-for-linux we have build_assert but it > is fragile and depends on the value being a constant. Since the setter API > accepts a run-time value and not a constant, we cannot use this. > > Or, we can fail at runtime, but that requires changing the set_* to try_set_* > and returning a Result instead of Self. Alternatively, we can have a debug > option that panics if the setter API is misued.Please no...> > Thoughts? > > Or for the moment, we can keep it simple and filter out / ignore extra bits > of the larger value passed (which is what nova-core's register macro bitfield > implementation currently does anyway). >Yes. Assuming that I'm not completely lost here, you are proposing to simply truncate to the size of the bitfield--no panics, no warnings. And that's perfectly fine here IMHO. thanks, -- John Hubbard
On Sat, Sep 20, 2025 at 2:39?AM Joel Fernandes <joelagnelf at nvidia.com> wrote:> > The C checks use BUILD_BUG_ON, in rust-for-linux we have build_assert but it > is fragile and depends on the value being a constant.What do you mean? `build_assert!` works essentially like `BUILD_BUG_ON`, i.e. after the optimizer, and does not depend on the value being a constant. You may be thinking of `static_assert!`, which is the compiler-based one. Cheers, Miguel