On Tue, 2 Nov 2021 09:43:16 +0200 Yordan Karadzhov <y.karadz@xxxxxxxxx> wrote: > > 2. Expose the dynevent_... APIs directly: > > struct tracefs_dynevent *synth = > > tracefs_dynevent_alloc(TRACE_DYNEVENT_SYNTH, ...); > > I would prefer taking this approach. It is OK to also have several helper constructors like tracefs_kprobe_alloc(), > tracefs_synth_alloc(), etc. that will call internally dynevent_alloc(). > > In principle having multiple constructors is OK. I agree with having multiple constructors, as that makes it easier to think about what is being created and how to manipulate it. The dynamic event structure has a type field and it should be used to make sure that modifications that only should happen to kprobes only happens on dynamic events created as a kprobe, and errors on everything else. > > > > struct tracefs_dynevent *kprobe = > > tracefs_dynevent_alloc(TRACE_DYNEVENT_KPROBE, ...); > > > > ret = tracefs_dynevent_create(synth); > > ret = tracefs_dynevent_create(kprobe); > > > > Yes, this is OK as well. Agreed. > > We only have a single type (struct tracefs_dynevent) so there must be a single create. And what is even more important: > there must be ONLY ONE destructor. I wouldn't be so strong as to use "must" but I do agree that it will simplify the usage of the API. It would be more analogous to free() where you have several different constructors that state in the man page (must be freed with free). Same here. Each constructor's man page must state: "must be freed with tracefs_dynevent_free()" Removing from the system any dynamic event should also just use tracefs_dynevent_destroy(). Having the type of event in the structure means that knowing how to destroy it can be multiplexed inside the library, and not burden the user in such cases. This way, the user could create a bunch of dynamic event types in an array or linked list, and destroy them at the end without having to keep track of which one is which. -- Steve > > Think about the usecase of the APIs. You create an object using one of the constructors. Later at some point you want to > use the objects and after this finally you want to destroy it. If we have multiple/specialized "create" and "destroy" > versions for the same object type, the user must somehow keep its own record of which constructor had been used trough > the entire live of the object in order to know which is the proper destructor to call at the end.
|