Merged in master, thanks! Jérémie
On Tue, Jun 05, 2018 at 12:11:20PM -0400, Michael Jeanson wrote: > readdir_r() has been deprecated since glibc 2.24 [1]. > > We used readdir_r() in load_session_from_path() to be thread-safe > since this function is part of liblttng-ust-ctl. However, according > to readdir()'s man page, it's thread-safe as long as the directory > stream it operates on is not shared across threads : > > In the current POSIX.1 specification (POSIX.1-2008), readdir(3) is > not required to be thread-safe. However, in modern > implementations (including the glibc implementation), concurrent > calls to readdir(3) that specify different directory streams are > thread-safe. Therefore, the use of readdir_r() is generally > unnecessary in multithreaded programs. In cases where multiple > threads must read from the same directory stream, using readdir(3) > with external synchronization is still preferable to the use of > readdir_r(), for the reasons given in the points above. > > In our use-case where we open the directory stream in the same function, > we know it won't be shared across threads and thus it's safe to use > readdir(). Here is the relevevant bit from the POSIX.1 [2] spec : > > The returned pointer, and pointers within the structure, might be > invalidated or the structure or the storage areas might be overwritten > by a subsequent call to readdir() on the same directory stream. They shall > not be affected by a call to readdir() on a different directory stream. > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=19056 > [2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/readdir.html > > Signed-off-by: Michael Jeanson <mjean...@efficios.com> > --- > src/common/config/session-config.c | 54 ++++++++++++++---------------- > 1 file changed, 26 insertions(+), 28 deletions(-) > > diff --git a/src/common/config/session-config.c > b/src/common/config/session-config.c > index 624064a1..56db1195 100644 > --- a/src/common/config/session-config.c > +++ b/src/common/config/session-config.c > @@ -2953,23 +2953,6 @@ end: > return ret; > } > > -/* Allocate dirent as recommended by READDIR(3), NOTES on readdir_r */ > -static > -struct dirent *alloc_dirent(const char *path) > -{ > - size_t len; > - long name_max; > - struct dirent *entry; > - > - name_max = pathconf(path, _PC_NAME_MAX); > - if (name_max == -1) { > - name_max = PATH_MAX; > - } > - len = offsetof(struct dirent, d_name) + name_max + 1; > - entry = zmalloc(len); > - return entry; > -} > - > static > int load_session_from_path(const char *path, const char *session_name, > struct session_config_validation_ctx *validation_ctx, int overwrite, > @@ -3006,7 +2989,6 @@ int load_session_from_path(const char *path, const char > *session_name, > } > } > if (directory) { > - struct dirent *entry; > struct dirent *result; > size_t file_path_root_len; > > @@ -3017,16 +2999,9 @@ int load_session_from_path(const char *path, const > char *session_name, > goto end; > } > > - entry = alloc_dirent(path); > - if (!entry) { > - ret = -LTTNG_ERR_NOMEM; > - goto end; > - } > - > ret = lttng_dynamic_buffer_append(&file_path, path, path_len); > if (ret) { > ret = -LTTNG_ERR_NOMEM; > - free(entry); > goto end; > } > > @@ -3040,8 +3015,32 @@ int load_session_from_path(const char *path, const > char *session_name, > file_path_root_len = file_path.size; > > /* Search for *.lttng files */ > - while (!readdir_r(directory, entry, &result) && result) { > - size_t file_name_len = strlen(result->d_name); > + for (;;) { > + size_t file_name_len; > + > + /* > + * When the end of the directory stream is reached, > NULL is > + * returned and errno is kept unchanged. When an error > occurs, > + * NULL is returned and errno is set accordingly. To > + * distinguish between the two, set errno to zero before > + * calling readdir(). > + * > + * On success, readdir() returns a pointer to a dirent > + * structure. (This structure may be statically > allocated, do > + * not attempt to free(3) it.) > + */ > + errno = 0; > + result = readdir(directory); > + > + /* Reached end of dir stream or error out */ > + if (!result) { > + if (errno) { > + PERROR("readdir"); > + } > + break; > + } > + > + file_name_len = strlen(result->d_name); > > if (file_name_len <= > sizeof(DEFAULT_SESSION_CONFIG_FILE_EXTENSION)) { > @@ -3089,7 +3088,6 @@ int load_session_from_path(const char *path, const char > *session_name, > } > } > > - free(entry); > } else { > ret = load_session_from_file(path, session_name, > validation_ctx, overwrite, overrides); > -- > 2.17.0 > _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
