On Tue, Oct 22, 2024 at 11:40 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > On Tue, Oct 22, 2024 at 10:29:13AM -0700, Andrii Nakryiko wrote: > > > > > > Would this work? > > > > > > #define SRCU_INVALID_INDEX -1 > > > > > > > But why? > > Becaue it very clearly documents what is going on. So does saying "returned value is going to be non-negative, always". It's not some weird and unusual thing in C by any means. > > >It's a nice property to have an int-returning API where valid > > values are only >= 0, so callers are free to use the entire negative > > range (not just -1) for whatever they need to store in case there is > > no srcu_idx value. > > Well, if you have a concrete use case for that we can probably live > with it, but I'd rather have that use case extremely well documented, > as it will be very puzzling to the reader. > See [0] for what Peter is proposing. Note hprobe_init() and its use of `srcu_idx < 0` as a mark that there is no SRCU "session" associated with the uprobe. Now, if we say that SRCU_INVALID_INDEX is the only invalid value, it leaves a question: what to do with other negative srcu_idx values? Are they valid? Should I now WARN() on "unknown invalid" values? Why all these complications? I'd rather just not have a unified hprobe_init() at that point, TBH. But anyways, taking a step back. Take idr_alloc()/idr_alloc_cyclic() APIs as an example. They return int, but valid IDs are documented to be positive. This leaves users of this API free to use int to store ID in their data structures, knowing that <= 0 is "no ID", and if necessary, they can have multiple possible "no ID" situations. Same principle here. Why prescribing a randomly picked -1 as the only "valid" invalid value, if anything negative is clearly impossible. [0] https://lore.kernel.org/linux-trace-kernel/20241018101647.GA36494@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
