On Wed, Feb 21, 2024 at 10:17:21AM -0500, Steven Rostedt wrote: > On Wed, 14 Feb 2024 17:50:43 +0000 > Beau Belgrave <be...@linux.microsoft.com> wrote: > > So the patches look good, but since I gave you some updates, I'm now going > to go though "nits". Like grammar and such ;-) >
Sure thing. > > The current code for finding and deleting events assumes that there will > > never be cases when user_events are registered with the same name, but > > different formats. In the future this scenario will exist to ensure > > > user programs can be updated or modify their events and run different > > versions of their programs side-by-side without being blocked. > > Can you change the last sentence above. I read it three times and it's > still awkward to understand it. Particularly, the "user programs can be > updated or modify their events". That just doesn't want to compute. > Yeah, I'll clean this up. > > > > This change does not yet allow for multi-format events. If user_events > > are registered with the same name but different arguments the programs > > see the same return values as before. This change simply makes it > > possible to easily accomodate for this in future changes. > > I think you can drop the "in future changes" part. > Agreed. > > > > Update find_user_event() to take in argument parameters and register > > flags to accomodate future multi-format event scenarios. Have find > > validate argument matching and return error pointers to cover address > > in use cases, or allocation errors. Update callers to handle error > > "to cover address in use cases" ? Yeah, if the ABI is using a single-format event and it's already in use, we return -EADDRINUSE. It does not happen in multi-format event cases, since that is allowed. I'll try to clarify this a bit. > > > pointer logic. > > > > Move delete_user_event() to use hash walking directly now that find has > > changed. Delete all events found that match the register name, stop > > "now that find has changed" ? You mean the "find function"? > Yeah, I'll just use the function name here, find_user_event(). > > if an error occurs and report back to the user. > > > > Update user_fields_match() to cover list_empty() scenarios instead of > > each callsite doing it now that find_user_event() uses it directly. > > The above is a bit of a run-on sentence. > I'll clean it up a bit. Thanks, -Beau > -- Steve > > > > > Signed-off-by: Beau Belgrave <be...@linux.microsoft.com> > > ---
