* [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() @ 2016-05-30 2:32 Chris Patterson 2016-05-30 2:32 ` [PATCH 2/2] libxl: " Chris Patterson 2016-05-31 10:42 ` [PATCH 1/2] libfsimage: " George Dunlap 0 siblings, 2 replies; 10+ messages in thread From: Chris Patterson @ 2016-05-30 2:32 UTC (permalink / raw) To: xen-devel; +Cc: wei.liu2, Chris Patterson, ian.jackson From: Chris Patterson <pattersonc@ainfosec.com> 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@ainfosec.com> --- 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@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] libxl: replace deprecated readdir_r() with readdir() 2016-05-30 2:32 [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() Chris Patterson @ 2016-05-30 2:32 ` Chris Patterson 2016-05-31 10:42 ` [PATCH 1/2] libfsimage: " George Dunlap 1 sibling, 0 replies; 10+ messages in thread From: Chris Patterson @ 2016-05-30 2:32 UTC (permalink / raw) To: xen-devel; +Cc: wei.liu2, Chris Patterson, ian.jackson From: Chris Patterson <pattersonc@ainfosec.com> 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 This also removes the zalloc_dirent() helper function which is no longer required. Signed-off-by: Chris Patterson <pattersonc@ainfosec.com> --- tools/libxl/libxl_pvusb.c | 26 ++++---------------------- tools/libxl/libxl_utils.c | 9 ++------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c index 9f1e842..d1397c4 100644 --- a/tools/libxl/libxl_pvusb.c +++ b/tools/libxl/libxl_pvusb.c @@ -508,19 +508,10 @@ int libxl_devid_to_device_usbctrl(libxl_ctx *ctx, return rc; } -static void *zalloc_dirent(libxl__gc *gc, const char *dirpath) -{ - size_t need = offsetof(struct dirent, d_name) + - pathconf(dirpath, _PC_NAME_MAX) + 1; - - return libxl__zalloc(gc, need); -} - static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr) { DIR *dir; char *busid = NULL; - struct dirent *de_buf; struct dirent *de; /* invalid hostbus or hostaddr */ @@ -533,21 +524,17 @@ static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr) return NULL; } - de_buf = zalloc_dirent(gc, SYSFS_USB_DEV); - for (;;) { char *filename; void *buf; int busnum = -1; int devnum = -1; - int r = readdir_r(dir, de_buf, &de); - if (r) { + de = readdir(dir); + if (!de) { LOGE(ERROR, "failed to readdir %s", SYSFS_USB_DEV); break; } - if (!de) - break; if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) @@ -1157,7 +1144,6 @@ static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid, { DIR *dir; char *buf; - struct dirent *de_buf; struct dirent *de; int rc; @@ -1172,18 +1158,14 @@ static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid, return ERROR_FAIL; } - de_buf = zalloc_dirent(gc, SYSFS_USB_DEV); - for (;;) { - int r = readdir_r(dir, de_buf, &de); + de = readdir(dir); - if (r) { + if (!de) { LOGE(ERROR, "failed to readdir %s", SYSFS_USB_DEV); rc = ERROR_FAIL; goto out; } - if (!de) - break; if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index ceb8825..613a9d6 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -548,20 +548,15 @@ int libxl__remove_directory(libxl__gc *gc, const char *dirpath) goto out; } - size_t need = offsetof(struct dirent, d_name) + - pathconf(dirpath, _PC_NAME_MAX) + 1; - struct dirent *de_buf = libxl__zalloc(gc, need); struct dirent *de; for (;;) { - int r = readdir_r(d, de_buf, &de); - if (r) { + de = readdir(d); + if (!de) { LOGE(ERROR, "failed to readdir %s for removal", dirpath); rc = ERROR_FAIL; break; } - if (!de) - break; if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-05-30 2:32 [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() Chris Patterson 2016-05-30 2:32 ` [PATCH 2/2] libxl: " Chris Patterson @ 2016-05-31 10:42 ` George Dunlap 2016-05-31 15:43 ` Chris Patterson 1 sibling, 1 reply; 10+ messages in thread From: George Dunlap @ 2016-05-31 10:42 UTC (permalink / raw) To: Chris Patterson; +Cc: Ian Jackson, Chris Patterson, Wei Liu, xen-devel On Mon, May 30, 2016 at 3:32 AM, Chris Patterson <cjp256@gmail.com> wrote: > From: Chris Patterson <pattersonc@ainfosec.com> > > 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@ainfosec.com> 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@lists.xen.org > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-05-31 10:42 ` [PATCH 1/2] libfsimage: " George Dunlap @ 2016-05-31 15:43 ` Chris Patterson 2016-05-31 17:53 ` Wei Liu 0 siblings, 1 reply; 10+ messages in thread From: Chris Patterson @ 2016-05-31 15:43 UTC (permalink / raw) To: George Dunlap; +Cc: Ian Jackson, Chris Patterson, Wei Liu, xen-devel On Tue, May 31, 2016 at 6:42 AM, George Dunlap <george.dunlap@citrix.com> wrote: > On Mon, May 30, 2016 at 3:32 AM, Chris Patterson <cjp256@gmail.com> wrote: >> From: Chris Patterson <pattersonc@ainfosec.com> >> >> 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@ainfosec.com> > > Thanks for the patch, Chris. A bit more background would have been > helpful -- I did some searching and found a description[1] which says: > Thank you. I should have added these details in the commit message or cover. > 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 Is there a list of supported libc libraries? I can look into it and provide more definitive answers if there are. > #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 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-05-31 15:43 ` Chris Patterson @ 2016-05-31 17:53 ` Wei Liu 2016-05-31 20:37 ` Chris Patterson 0 siblings, 1 reply; 10+ messages in thread From: Wei Liu @ 2016-05-31 17:53 UTC (permalink / raw) To: Chris Patterson Cc: Ian Jackson, Chris Patterson, Wei Liu, George Dunlap, xen-devel On Tue, May 31, 2016 at 11:43:13AM -0400, Chris Patterson wrote: > On Tue, May 31, 2016 at 6:42 AM, George Dunlap <george.dunlap@citrix.com> wrote: > > On Mon, May 30, 2016 at 3:32 AM, Chris Patterson <cjp256@gmail.com> wrote: > >> From: Chris Patterson <pattersonc@ainfosec.com> > >> > >> 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@ainfosec.com> > > > > Thanks for the patch, Chris. A bit more background would have been > > helpful -- I did some searching and found a description[1] which says: > > > > Thank you. I should have added these details in the commit message or cover. > > > 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 > > Is there a list of supported libc libraries? I can look into it and > provide more definitive answers if there are. > We at least care about FreeBSD and NetBSD. Sadly their manuaks don't provide statement regarding thread safety for readdir and readdir_r. It's better to err on the safe side. Wei. > > #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 > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-05-31 17:53 ` Wei Liu @ 2016-05-31 20:37 ` Chris Patterson 2016-06-01 11:06 ` Ian Jackson 0 siblings, 1 reply; 10+ messages in thread From: Chris Patterson @ 2016-05-31 20:37 UTC (permalink / raw) To: Wei Liu; +Cc: Chris Patterson, Ian Jackson, George Dunlap, xen-devel On Tue, May 31, 2016 at 1:53 PM, Wei Liu <wei.liu2@citrix.com> wrote: > On Tue, May 31, 2016 at 11:43:13AM -0400, Chris Patterson wrote: >> On Tue, May 31, 2016 at 6:42 AM, George Dunlap <george.dunlap@citrix.com> wrote: >> > On Mon, May 30, 2016 at 3:32 AM, Chris Patterson <cjp256@gmail.com> wrote: >> >> From: Chris Patterson <pattersonc@ainfosec.com> >> >> >> >> 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@ainfosec.com> >> > >> > Thanks for the patch, Chris. A bit more background would have been >> > helpful -- I did some searching and found a description[1] which says: >> > >> >> Thank you. I should have added these details in the commit message or cover. >> >> > 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 >> >> Is there a list of supported libc libraries? I can look into it and >> provide more definitive answers if there are. >> > > We at least care about FreeBSD and NetBSD. Sadly their manuaks don't > provide statement regarding thread safety for readdir and readdir_r. > It's better to err on the safe side. > I'm far from the expert here, but it would appear that both NetBSD's and FreeBSD's libc readdir()/readdir_r() implementations are consistent in their locking mechanisms [1,2]. As such, I would expect readdir() to be equally as safe as readdir_r() for their users. As you noted, there does not appear to be any documented distinction within their man pages [3,4] with regards to thread safety and it seems reasonable that they would have documented these limitations, if present. [1] FreeBSD: https://svnweb.freebsd.org/base/head/lib/libc/gen/readdir.c?view=markup#l98 [2] NetBSD: http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/gen/readdir.c?rev=1.25.6.1&content-type=text/x-cvsweb-markup [3] FreeBSD readdir manpage: https://www.freebsd.org/cgi/man.cgi?query=readdir§ion=3 [4] NetBSD readdir manpage: http://netbsd.gw.com/cgi-bin/man-cgi?readdir Are there other known supported libc implementations? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-05-31 20:37 ` Chris Patterson @ 2016-06-01 11:06 ` Ian Jackson 2016-06-01 12:04 ` Ian Jackson 0 siblings, 1 reply; 10+ messages in thread From: Ian Jackson @ 2016-06-01 11:06 UTC (permalink / raw) To: Chris Patterson; +Cc: Chris Patterson, Wei Liu, George Dunlap, xen-devel Chris Patterson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()"): > I'm far from the expert here, but it would appear that both NetBSD's > and FreeBSD's libc readdir()/readdir_r() implementations are > consistent in their locking mechanisms [1,2]. As such, I would expect > readdir() to be equally as safe as readdir_r() for their users. As > you noted, there does not appear to be any documented distinction > within their man pages [3,4] with regards to thread safety and it > seems reasonable that they would have documented these limitations, if > present. Thanks for chasing up this breakage. However, I think that: 1. This is a fundamentally wrong way to go about assessing the proper way to use a supposedly-portable API. The right way is to refer to the published specification, in the first instance. That published specification says that readdir() is not threadsafe. 2. There may be good reasons to deviate from a formal specification. Formal specifications can be wrong (for example, they can differ from established practice, or unuseable, or incoherent). But there has been no discussion (at least in this thread on xen-devel) which might suggest that the POSIX specification is wrongheaded here. 3. Perhaps the documentation accompanying, or discussion justifying, the glibc readdir deprecation warning, will provide something resembling what I discuss in (2). 4. I am not satisfied with an approach which enumerates all the currently-supported dom0 operating systems. These patches should be accompanied by an explantion of a good reason to believe that all operating systems we are likely to ever want to run on (or be able to run on) will also provide a threadsafe readdir. 5. I am not opposed to replacing readdir_r with readdir in areas of code which do not need the threadsafety properties. (Eg, in xl.) Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-06-01 11:06 ` Ian Jackson @ 2016-06-01 12:04 ` Ian Jackson 2016-06-01 12:06 ` Ian Jackson 0 siblings, 1 reply; 10+ messages in thread From: Ian Jackson @ 2016-06-01 12:04 UTC (permalink / raw) To: Chris Patterson, Wei Liu, George Dunlap, xen-devel, Chris Patterson Ian Jackson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()"): > 2. There may be good reasons to deviate from a formal specification. > Formal specifications can be wrong (for example, they can differ from > established practice, or unuseable, or incoherent). But there has > been no discussion (at least in this thread on xen-devel) which might > suggest that the POSIX specification is wrongheaded here. I have been helpfully referred by a local irc channel to the following attempt to change posix to require that readdir() is threadsafe in the senses required by libx, and to deprecate readdir_r(): http://austingroupbugs.net/view.php?id=696 I find the comment 0001606 by "dalias" (et seq) totally convincing. The published specification of readdir_r is indeed incoherent. And only contrived implementations of readdir will not be threadsafe in the required sense. > 3. Perhaps the documentation accompanying, or discussion justifying, > the glibc readdir deprecation warning, will provide something > resembling what I discuss in (2). > > 4. I am not satisfied with an approach which enumerates all the > currently-supported dom0 operating systems. These patches should be > accompanied by an explantion of a good reason to believe that all > operating systems we are likely to ever want to run on (or be able to > run on) will also provide a threadsafe readdir. I think the comments by dalias in the thread above, provide exactly the justification I wanted in my paragraphs 2-4 above. Accordingly, I think all occurrences of readdir_r in our codebase should be replaced by readdir, as proposed by Chris. However, I think the patch is not quite complete, as the change from readdir_r to readdir should also involve removing the local dirent variables associated with each call site. Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-06-01 12:04 ` Ian Jackson @ 2016-06-01 12:06 ` Ian Jackson 2016-06-01 13:55 ` Chris Patterson 0 siblings, 1 reply; 10+ messages in thread From: Ian Jackson @ 2016-06-01 12:06 UTC (permalink / raw) To: Chris Patterson, Wei Liu, George Dunlap, xen-devel, Chris Patterson Ian Jackson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()"): > Ian Jackson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()"): > > 2. There may be good reasons to deviate from a formal specification. > > Formal specifications can be wrong (for example, they can differ from > > established practice, or unuseable, or incoherent). But there has > > been no discussion (at least in this thread on xen-devel) which might > > suggest that the POSIX specification is wrongheaded here. > > I have been helpfully referred by a local irc channel to the following > attempt to change posix to require that readdir() is threadsafe in the > senses required by libx, and to deprecate readdir_r(): > > http://austingroupbugs.net/view.php?id=696 > > I find the comment 0001606 by "dalias" (et seq) totally convincing. > The published specification of readdir_r is indeed incoherent. And > only contrived implementations of readdir will not be threadsafe in > the required sense. ... > Accordingly, I think all occurrences of readdir_r in our codebase > should be replaced by readdir, as proposed by Chris. > > However, I think the patch is not quite complete, as the change from > readdir_r to readdir should also involve removing the local dirent > variables associated with each call site. Also, the commit message needs to be expanded to provide the rationale. It should restate the reasoning provided by "dalias" and provide links to the austingroupbugs thread and references to the comment numbers. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() 2016-06-01 12:06 ` Ian Jackson @ 2016-06-01 13:55 ` Chris Patterson 0 siblings, 0 replies; 10+ messages in thread From: Chris Patterson @ 2016-06-01 13:55 UTC (permalink / raw) To: Ian Jackson; +Cc: Chris Patterson, Wei Liu, George Dunlap, xen-devel On Wed, Jun 1, 2016 at 8:06 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote: > Ian Jackson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()"): >> Ian Jackson writes ("Re: [Xen-devel] [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir()"): >> > 2. There may be good reasons to deviate from a formal specification. >> > Formal specifications can be wrong (for example, they can differ from >> > established practice, or unuseable, or incoherent). But there has >> > been no discussion (at least in this thread on xen-devel) which might >> > suggest that the POSIX specification is wrongheaded here. >> >> I have been helpfully referred by a local irc channel to the following >> attempt to change posix to require that readdir() is threadsafe in the >> senses required by libx, and to deprecate readdir_r(): >> >> http://austingroupbugs.net/view.php?id=696 >> >> I find the comment 0001606 by "dalias" (et seq) totally convincing. >> The published specification of readdir_r is indeed incoherent. And >> only contrived implementations of readdir will not be threadsafe in >> the required sense. > ... >> Accordingly, I think all occurrences of readdir_r in our codebase >> should be replaced by readdir, as proposed by Chris. >> >> However, I think the patch is not quite complete, as the change from >> readdir_r to readdir should also involve removing the local dirent >> variables associated with each call site. > > Also, the commit message needs to be expanded to provide the > rationale. It should restate the reasoning provided by "dalias" and > provide links to the austingroupbugs thread and references to the > comment numbers. > > Ian. Agreed, will post a v2. Thanks for the feedback. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-06-01 13:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-30 2:32 [PATCH 1/2] libfsimage: replace deprecated readdir_r() with readdir() Chris Patterson 2016-05-30 2:32 ` [PATCH 2/2] libxl: " Chris Patterson 2016-05-31 10:42 ` [PATCH 1/2] libfsimage: " George Dunlap 2016-05-31 15:43 ` Chris Patterson 2016-05-31 17:53 ` Wei Liu 2016-05-31 20:37 ` Chris Patterson 2016-06-01 11:06 ` Ian Jackson 2016-06-01 12:04 ` Ian Jackson 2016-06-01 12:06 ` Ian Jackson 2016-06-01 13:55 ` Chris Patterson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.