On Sun, Apr 26, 2020 at 8:53 AM Dmitry Kozlyuk <dmitry.kozl...@gmail.com> wrote: > > Add EAL private functions to support trace storage: > > * eal_persistent_data_path() > * eal_dir_create() > > Replace clock_gettime(CLOCK_REALTIME) with C11 timespec_get(). > Implementation is provided for MinGW-w64 that misses this function. > > Provide minimum viable implementations of malloc and timer functions > used by tracing. > > Fixes: 185b7dc1d467 ("trace: save bootup timestamp") > Fixes: 321dd5f8fa62 ("trace: add internal init and fini interface") > Reported-by: Pallavi Kadam <pallavi.ka...@intel.com> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozl...@gmail.com> > ---
In general, the patch looks good to me. Could you split the patch as two 1) New eal_permanent_data_path() and eal_dir_create() API definition, and its implementation with unix and the common code changes to adapt new APIs. 2) Windows implementation of eal_permanent_data_path() and other missing pieces for trace implementation. Some comments below, > > void eal_free_no_trace(void *addr); > > +/** > + * Get absolute path to the directory where permanent data can be stored. > + * > + * @return > + * Statically allocated string on success, NULL on failure. > + */ > +const char * > +eal_permanent_data_path(void); Do windows have PATH_MAX kind of macro? I think, it is better API consumer allocates the memory of size PATH_MAX and implementation fills it, instead of, the static scheme. > + > +/** > + * Create a directory accessible to the current user only. > + * > + * This function does not create intermediate directories, > + * thus only the last path component may be nonexistent. > + * > + * This function succeeds if path already exists and is a directory. > + * > + * Platform-independent code should use forward slash as path separator. > + * > + * @param path > + * Path to be created. > + * @return > + * 0 on success, (-1) on failure and rte_errno is set. > + */ > +int eal_dir_create(const char *path); > Looks good to me.