Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On Mon, May 30, 2016 at 3:32 AM, Chris Patterson <cjp256@xxxxxxxxx> wrote:
> From: Chris Patterson <pattersonc@xxxxxxxxxxxx>
>
> Replace the usage of readdir_r() with readdir() to address
> a compilation error due to the deprecation of readdir_r.
>
> glibc has deprecated this for their next release (2.24):
> https://sourceware.org/bugzilla/show_bug.cgi?id=19056
>
> Signed-off-by: Chris Patterson <pattersonc@xxxxxxxxxxxx>

Thanks for the patch, Chris.  A bit more background would have been
helpful -- I did some searching and found a description[1] which says:

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.

The use of the specific directory stream is single-threaded, so for
glibc, it looks like using readdir() will be safe.  But libxl needs to
be able to build on a number of libc's that are not glibc and still be
thread-safe.  So we need to either 1) verify that readdir() is thread
safe on all libc's against which we may compile, or 2) add some
#ifdef-ery to switch to readdir_r() on systems unless we know it's
thread-safe.

I'm less familiar with best practices for this kind of thing -- Ian,
do you have an idea?

Thanks again for raising this, Chris.

 -George

[1] http://man7.org/linux/man-pages/man3/readdir_r.3.html

> ---
>  tools/libfsimage/common/fsimage_plugin.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/libfsimage/common/fsimage_plugin.c 
> b/tools/libfsimage/common/fsimage_plugin.c
> index 3fa06c7..03212e1 100644
> --- a/tools/libfsimage/common/fsimage_plugin.c
> +++ b/tools/libfsimage/common/fsimage_plugin.c
> @@ -147,7 +147,7 @@ static int load_plugins(void)
>
>         bzero(dp, sizeof (struct dirent) + name_max + 1);
>
> -       while (readdir_r(dir, dp, &dpp) == 0 && dpp != NULL) {
> +       while ((dpp = readdir(dir)) != NULL) {
>                 if (strcmp(dpp->d_name, ".") == 0)
>                         continue;
>                 if (strcmp(dpp->d_name, "..") == 0)
> --
> 2.1.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel