| Message ID | 20210610113426.257931-7-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add trace file compression | expand |
On Thu, 10 Jun 2021 14:33:52 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > The trace-cmd library has no APIs for initialization and free of the > whole library. Added these new APIs: > tracecmd_lib_init() > tracecmd_lib_free() This isn't a very use friendly API. The reason I see you added this was for zlib, which I don't think is necessary. -- Steve > and call them in trace-cmd main function. >
On Tue, Jun 22, 2021 at 4:27 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Thu, 10 Jun 2021 14:33:52 +0300 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > The trace-cmd library has no APIs for initialization and free of the > > whole library. Added these new APIs: > > tracecmd_lib_init() > > tracecmd_lib_free() > > This isn't a very use friendly API. > > The reason I see you added this was for zlib, which I don't think is > necessary. I was missing that API when I worked on the host-guest time synch algorithms, to initialize the library time synchronization context. I decided to introduce a new API, for initializing only that part of the library - tracecmd_tsync_init(), as this functionality will not be used commonly. Now with the compression changes, there is a need for an API for initializing the library compression context. As this functionality will be used in almost all use cases, I think it makes sense to introduce this global tracecmd_lib_init() API, instead of something like tracecmd_lib_compression_init(). Personally I do not like the approach to have init APIs for different parts of the library, especially in case of widely used functionality. This will complicate the usage and will force the users to know the library internals. I would even suggest to remove the tracecmd_tsync_init() API and to init time sync context in the tracecmd_lib_init(). > > -- Steve > > > > > and call them in trace-cmd main function. > > -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center
On Tue, 22 Jun 2021 09:41:20 +0300 Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > I was missing that API when I worked on the host-guest time synch > algorithms, to initialize the library time synchronization context. I > decided to introduce a new API, for initializing only that part of the > library - tracecmd_tsync_init(), as this functionality will not be > used commonly. Now with the compression changes, there is a need for > an API for initializing the library compression context. As this I don't see that, but I'll reply to the other email on my thoughts there. > functionality will be used in almost all use cases, I think it makes > sense to introduce this global tracecmd_lib_init() API, instead of > something like tracecmd_lib_compression_init(). Personally I do not > like the approach to have init APIs for different parts of the > library, especially in case of widely used functionality. This will > complicate the usage and will force the users to know the library > internals. I would even suggest to remove the tracecmd_tsync_init() > API and to init time sync context in the tracecmd_lib_init(). The tsync_init() is a specific functionality and only for recording, but a lib_init() sounds very generic, and something that needs to be run if needed or not. I still don't see the need for it. -- Steve
diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h index 01b12c47..be8b3c48 100644 --- a/lib/trace-cmd/include/private/trace-cmd-private.h +++ b/lib/trace-cmd/include/private/trace-cmd-private.h @@ -29,6 +29,9 @@ struct tep_plugin_list *trace_load_plugins(struct tep_handle *tep, int flags); +int tracecmd_lib_init(void); +void tracecmd_lib_free(void); + int *tracecmd_add_id(int *list, int id, int len); enum { diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index b0c98c72..61054ad2 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -624,3 +624,12 @@ bool tracecmd_is_version_supported(unsigned int version) return true; return false; } + +int tracecmd_lib_init(void) +{ + return 0; +} + +void tracecmd_lib_free(void) +{ +} diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c index 00cdaa37..71c8f6d6 100644 --- a/tracecmd/trace-cmd.c +++ b/tracecmd/trace-cmd.c @@ -142,15 +142,20 @@ int main (int argc, char **argv) if (argc < 2) trace_usage(argc, argv); + tracecmd_lib_init(); + for (i = 0; i < ARRAY_SIZE(commands); ++i) { if (strcmp(argv[1], commands[i].name) == 0 ){ commands[i].run(argc, argv); - goto out; + break; } } + tracecmd_lib_free(); + /* No valid command found, show help */ - trace_usage(argc, argv); -out: + if (i == ARRAY_SIZE(commands)) + trace_usage(argc, argv); + exit(0); }
The trace-cmd library has no APIs for initialization and free of the whole library. Added these new APIs: tracecmd_lib_init() tracecmd_lib_free() and call them in trace-cmd main function. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- lib/trace-cmd/include/private/trace-cmd-private.h | 3 +++ lib/trace-cmd/trace-util.c | 9 +++++++++ tracecmd/trace-cmd.c | 11 ++++++++--- 3 files changed, 20 insertions(+), 3 deletions(-)