core: use readdir(3) with glibc, and associated cleanup

Starting with glibc-2.23 (i.e. what's in Fedora 25), readdir_r(3)
is marked as deprecated. Specifically the function decl in <dirent.h>
has the deprecated attribute, and now warnings are thrown during the
compile on Fedora 25 builds.

The readdir(_r)(3) man page (on Fedora 25 at least) and World+Dog say
that glibc's readdir(3) is, and always has been, MT-SAFE as long as
only one thread is accessing the directory object returned by opendir().
World+Dog also says there is a potential buffer overflow in readdir_r().
World+Dog suggests that it is preferable to simply use readdir(). There's
an implication that eventually readdir_r(3) will be removed from glibc.
POSIX has, apparently deprecated it in the standard, or even removed it
entirely.

Over and above that, our source near the various uses of readdir(_r)(3)
has a few unsafe uses of strcpy()+strcat().

(AFAIK nobody has looked at the readdir(3) implemenation in *BSD to see
if the same is true on those platforms, and we can't be sure of MacOS
even though we know it's based on *BSD.)

Change-Id: I5481f18ba1eebe7ee177895eecc9a80a71b60568
BUG: 1356998
Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com>
Reviewed-on: http://review.gluster.org/14838
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Niels de Vos <ndevos@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Kotresh HR <khiremat@redhat.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>

2 files changed, 51 insertions, 50 deletions

diff --git a/xlators/features/changelog/lib/src/gf-changelog-api.c b/xlators/features/changelog/lib/src/gf-changelog-api.c
index f41b505a749..d2a28bc6d52 100644
--- a/xlators/features/changelog/lib/src/gf-changelog-api.c
+++ b/xlators/features/changelog/lib/src/gf-changelog-api.c
@@ -152,16 +152,16 @@ out:
 ssize_t
 gf_changelog_scan ()
 {
-        int             ret        = 0;
-        int             tracker_fd = 0;
-        size_t          len        = 0;
-        size_t          off        = 0;
-        xlator_t       *this       = NULL;
-        size_t          nr_entries = 0;
+        int             ret         = 0;
+        int             tracker_fd  = 0;
+        size_t          len         = 0;
+        size_t          off         = 0;
+        xlator_t       *this        = NULL;
+        size_t          nr_entries  = 0;
         gf_changelog_journal_t *jnl = NULL;
-        struct dirent  *entryp     = NULL;
-        struct dirent  *result     = NULL;
-        char buffer[PATH_MAX]      = {0,};
+        struct dirent  *entry       = NULL;
+        struct dirent   scratch[2]  = {{0,},};
+        char            buffer[PATH_MAX] = {0,};
 
         this = THIS;
         if (!this)
@@ -183,19 +183,17 @@ gf_changelog_scan ()
 
         len = offsetof(struct dirent, d_name)
                 + pathconf(jnl->jnl_processing_dir, _PC_NAME_MAX) + 1;
-        entryp = GF_CALLOC (1, len,
-                            gf_changelog_mt_libgfchangelog_dirent_t);
-        if (!entryp)
-                goto out;
 
         rewinddir (jnl->jnl_dir);
-        while (1) {
-                ret = readdir_r (jnl->jnl_dir, entryp, &result);
-                if (ret || !result)
+
+        for (;;) {
+                errno = 0;
+                entry = sys_readdir (jnl->jnl_dir, scratch);
+                if (!entry || errno != 0)
                         break;
 
-                if (!strcmp (basename (entryp->d_name), ".")
-                     || !strcmp (basename (entryp->d_name), ".."))
+                if (!strcmp (basename (entry->d_name), ".")
+                     || !strcmp (basename (entry->d_name), ".."))
                         continue;
 
                 nr_entries++;
@@ -203,8 +201,8 @@ gf_changelog_scan ()
                 GF_CHANGELOG_FILL_BUFFER (jnl->jnl_processing_dir,
                                           buffer, off,
                                           strlen (jnl->jnl_processing_dir));
-                GF_CHANGELOG_FILL_BUFFER (entryp->d_name, buffer,
-                                          off, strlen (entryp->d_name));
+                GF_CHANGELOG_FILL_BUFFER (entry->d_name, buffer,
+                                          off, strlen (entry->d_name));
                 GF_CHANGELOG_FILL_BUFFER ("\n", buffer, off, 1);
 
                 if (gf_changelog_write (tracker_fd, buffer, off) != off) {
@@ -217,9 +215,7 @@ gf_changelog_scan ()
                 off = 0;
         }
 
-        GF_FREE (entryp);
-
-        if (!result) {
+        if (!entry) {
                 if (gf_lseek (tracker_fd, 0, SEEK_SET) != -1)
                         return nr_entries;
         }
diff --git a/xlators/features/changelog/lib/src/gf-history-changelog.c b/xlators/features/changelog/lib/src/gf-history-changelog.c
index 389b65f3b16..5ed50390a7c 100644
--- a/xlators/features/changelog/lib/src/gf-history-changelog.c
+++ b/xlators/features/changelog/lib/src/gf-history-changelog.c
@@ -222,8 +222,8 @@ gf_history_changelog_scan ()
         size_t                  nr_entries   = 0;
         gf_changelog_journal_t *jnl          = NULL;
         gf_changelog_journal_t *hist_jnl     = NULL;
-        struct dirent          *entryp       = NULL;
-        struct dirent          *result       = NULL;
+        struct dirent          *entry        = NULL;
+        struct dirent           scratch[2]   = {{0,},};
         char buffer[PATH_MAX]                = {0,};
         static int              is_last_scan;
 
@@ -260,19 +260,17 @@ gf_history_changelog_scan ()
 
         len = offsetof (struct dirent, d_name)
                 + pathconf (hist_jnl->jnl_processing_dir, _PC_NAME_MAX) + 1;
-        entryp = GF_CALLOC (1, len,
-                            gf_changelog_mt_libgfchangelog_dirent_t);
-        if (!entryp)
-                goto out;
 
         rewinddir (hist_jnl->jnl_dir);
-        while (1) {
-                ret = readdir_r (hist_jnl->jnl_dir, entryp, &result);
-                if (ret || !result)
+
+        for (;;) {
+                errno = 0;
+                entry = sys_readdir (hist_jnl->jnl_dir, scratch);
+                if (!entry || errno != 0)
                         break;
 
-                if ( !strcmp (basename (entryp->d_name), ".")
-                     || !strcmp (basename (entryp->d_name), "..") )
+                if (strcmp (basename (entry->d_name), ".") == 0 ||
+                    strcmp (basename (entry->d_name), "..") == 0)
                         continue;
 
                 nr_entries++;
@@ -280,8 +278,8 @@ gf_history_changelog_scan ()
                 GF_CHANGELOG_FILL_BUFFER (hist_jnl->jnl_processing_dir,
                                           buffer, off,
                                           strlen (hist_jnl->jnl_processing_dir));
-                GF_CHANGELOG_FILL_BUFFER (entryp->d_name, buffer,
-                                          off, strlen (entryp->d_name));
+                GF_CHANGELOG_FILL_BUFFER (entry->d_name, buffer,
+                                          off, strlen (entry->d_name));
                 GF_CHANGELOG_FILL_BUFFER ("\n", buffer, off, 1);
 
                 if (gf_changelog_write (tracker_fd, buffer, off) != off) {
@@ -294,13 +292,11 @@ gf_history_changelog_scan ()
                 off = 0;
         }
 
-        GF_FREE (entryp);
-
         gf_msg_debug (this->name, 0,
                       "hist_done %d, is_last_scan: %d",
                       hist_jnl->hist_done, is_last_scan);
 
-        if (!result) {
+        if (!entry) {
                 if (gf_lseek (tracker_fd, 0, SEEK_SET) != -1) {
                         if (nr_entries > 0)
                                 return nr_entries;
@@ -678,7 +674,7 @@ gf_history_consume (void * data)
 
 out:
         if (fd != -1)
-                sys_close (fd);
+                (void) sys_close (fd);
         GF_FREE (hist_data);
         return NULL;
 }
@@ -794,12 +790,13 @@ gf_history_changelog (char* changelog_dir, unsigned long start,
         unsigned long                   to                      = 0;
         unsigned long                   from                    = 0;
         unsigned long                   total_changelog         = 0;
-        xlator_t                        *this                   = NULL;
-        gf_changelog_journal_t                  *jnl                    = NULL;
-        gf_changelog_journal_t                  *hist_jnl               = NULL;
-        gf_changelog_history_data_t     *hist_data              = NULL;
-        DIR                             *dirp                   = NULL;
-        struct dirent                   *dp                     = NULL;
+        xlator_t                       *this                    = NULL;
+        gf_changelog_journal_t         *jnl                     = NULL;
+        gf_changelog_journal_t         *hist_jnl                = NULL;
+        gf_changelog_history_data_t    *hist_data               = NULL;
+        DIR                            *dirp                    = NULL;
+        struct dirent                  *entry                   = NULL;
+        struct dirent                   scratch[2]              = {{0,},};
         pthread_t                       consume_th              = 0;
         char                            htime_dir[PATH_MAX]     = {0,};
         char                            buffer[PATH_MAX]        = {0,};
@@ -851,8 +848,16 @@ gf_history_changelog (char* changelog_dir, unsigned long start,
                 goto out;
         }
 
-        while ((dp = sys_readdir (dirp)) != NULL) {
-                ret = gf_changelog_extract_min_max (dp->d_name, htime_dir,
+        for (;;) {
+
+                errno = 0;
+
+                entry = sys_readdir (dirp, scratch);
+
+                if (!entry || errno != 0)
+                        break;
+
+                ret = gf_changelog_extract_min_max (entry->d_name, htime_dir,
                                                     &fd, &total_changelog,
                                                     &min_ts, &max_ts);
                 if (ret) {
@@ -968,11 +973,11 @@ gf_history_changelog (char* changelog_dir, unsigned long start,
 
 out:
         if (dirp != NULL)
-                sys_closedir (dirp);
+                (void) sys_closedir (dirp);
 
         if (ret < 0) {
                 if (fd != -1)
-                        sys_close (fd);
+                        (void) sys_close (fd);
                 GF_FREE (hist_data);
                 (void) pthread_attr_destroy (&attr);