[lttng-dev] [PATCH lttng-tools] Replace deprecated readdir_r() with readdir()

[Jérémie Galarneau]

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 <mjeanson at 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
>