# /win32/readdir.c review

- Thread: https://externals.io/message/10461
- Original archive: https://news-web.php.net/php.internals/10461
- Started by Alexander Valyalkin on 2004-06-15T10:33:30+00:00
- Messages: 3
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #10461 — Alexander Valyalkin — 2004-06-15T10:33:30+00:00

URL: https://externals.io/message/10461#10461

Today I checked file /win32/readdir.c
Below you can view its source with my comments.

================begin of /win32/readdir.c source===============
DIR *opendir(const char *dir)
{
     DIR *dp;
     char *filespec;
     long handle;
     /* - [index] must have (size_t) type (see it usage below)
     */
     int index;

     /* - there is no type casting from (void *) to (char *)
        - there is no error check after malloc
        - why do not use emalloc according to CODING_STANDARDS?
      */
     filespec = malloc(strlen(dir) + 2 + 1);
     strcpy(filespec, dir);
     /* - function `strlen()` returns value of type (size_t), not (int)
        - is strlen(dir) != strlen(filespec) ?
     */
     index = strlen(filespec) - 1;
     /* - memory access violation if dir == '\\'
        - wrong behaviour if dir == 'something/////'. All trailing slashes  
must be deleted
        - CODING_STANDARDS violation
     */
     if (index >= 0 && (filespec[index] == '/' ||
        (filespec[index] == '\\' && !IsDBCSLeadByte(filespec[index-1]))))
         filespec[index] = '\0';
     strcat(filespec, "/*");

     /* - there is no error check after malloc
        - why do not use emalloc according to CODING_STANDARDS?
     */
     dp = (DIR *) malloc(sizeof(DIR));
     dp->offset = 0;
     dp->finished = 0;
     /* - there is no error check after strdup
     */
     dp->dir = strdup(dir);

     if ((handle = _findfirst(filespec, &(dp->fileinfo))) < 0) {
     /* - CODING_STANDARDS violation
     */
         if (errno == ENOENT)
             dp->finished = 1;
     /* - CODING_STANDARDS violation
        - memory leak
     */
         else
             return NULL;
     }
     dp->handle = handle;
     free(filespec);

     return dp;
}

struct dirent *readdir(DIR *dp)
{
     /* - CODING_STANDARDS violation
        - are you sure that `NULL` == 0 on any system ?
     */
     if (!dp || dp->finished)
         return NULL;

     if (dp->offset != 0) {
         if (_findnext(dp->handle, &(dp->fileinfo)) < 0) {
             dp->finished = 1;
             return NULL;
         }
     }
     dp->offset++;

     /* - is strlen(dp->dent.d_name) != strlen(dp->fileinfo.name) ?
          faster way is:
            str_len = strlen(dp->fileinfo.name);
            str_len = str_len > _MAX_FNAME ? _MAX_FNAME : str_len;
            memcpy(dp->dent.d_name, dp->fileinfo.name, str_len);
            dp->dent.d_name[str_len] = '\0';
            dp->dent.d_reclen = str_len;
     */
     strlcpy(dp->dent.d_name, dp->fileinfo.name, _MAX_FNAME+1);
     dp->dent.d_ino = 1;
     dp->dent.d_reclen = strlen(dp->dent.d_name);
     dp->dent.d_off = dp->offset;

     return &(dp->dent);
}

int readdir_r(DIR *dp, struct dirent *entry, struct dirent **result)
{
     /* - are you sure that `NULL` == 0 on any system ?
     */
     if (!dp || dp->finished) {
         *result = NULL;
         return 0;
     }

     if (dp->offset != 0) {
         if (_findnext(dp->handle, &(dp->fileinfo)) < 0) {
             dp->finished = 1;
             *result = NULL;
             return 0;
         }
     }
     dp->offset++;

     /* - same as in `readdir()`
     */
     strlcpy(dp->dent.d_name, dp->fileinfo.name, _MAX_FNAME+1);
     dp->dent.d_ino = 1;
     dp->dent.d_reclen = strlen(dp->dent.d_name);
     dp->dent.d_off = dp->offset;

     memcpy(entry, &dp->dent, sizeof(*entry));

     *result = &dp->dent;

     return 0;
}

int closedir(DIR *dp)
{
     /* - CODING_STADNARDS violation
        - is `NULL` == 0 ?
     */
     if (!dp)
         return 0;
     _findclose(dp->handle);
     if (dp->dir)
     /* - why dont use efree according to CODING_STANDARDS ?
     */
         free(dp->dir);
     if (dp)
         free(dp);

     return 0;
}

int rewinddir(DIR *dp)
{
     /* Re-set to the beginning */
     char *filespec;
     long handle;
     /* - [index] must be (size_t) type
     */
     int index;

     _findclose(dp->handle);

     dp->offset = 0;
     dp->finished = 0;

     /* - missing type casting from (void *) to (char *)
        - missing error check after malloc
        - CODING_STANDARDS violation. Why dont use emalloc ?
     */
     filespec = malloc(strlen(dp->dir) + 2 + 1);
     strcpy(filespec, dp->dir);
     /* - missing type casting from (size_t) to (int)
     */
     index = strlen(filespec) - 1;
     /* - were is IsDBCSLeadByte() check ?
        - CODING_STANDARDS violation
        - wrong behaviour if filespec = 'something/////'
     */
     if (index >= 0 && (filespec[index] == '/' || filespec[index] == '\\'))
         filespec[index] = '\0';
     strcat(filespec, "/*");

     if ((handle = _findfirst(filespec, &(dp->fileinfo))) < 0) {
     /* - CODING_STANDARDS violation
     */
         if (errno == ENOENT)
             dp->finished = 1;
         }
     dp->handle = handle;
     free(filespec);
     /* - where is tab ? */
return 0;
}
================end of /win32/readdir.c source===============

-- 
Using Opera's revolutionary e-mail client: http://www.opera.com/m2/

---

## #10462 — Alexander Valyalkin — 2004-06-15T10:37:01+00:00

URL: https://externals.io/message/10461#10462 · In reply to: #10461

Below I provide my version of readdir.c file

unified diff
============cut===========
--- readdir.c   Tue Jan 28 04:51:26 2003
+++ readdir_new.c   Tue Jun 15 13:35:43 2004
@@ -22,40 +22,80 @@
  DIR *opendir(const char *dir)
  {
     DIR *dp;
-   char *filespec;
+    char *filespec, *tmp_ptr;
     long handle;
-   int index;
-
-   filespec = malloc(strlen(dir) + 2 + 1);
-   strcpy(filespec, dir);
-   index = strlen(filespec) - 1;
-   if (index >= 0 && (filespec[index] == '/' ||
-      (filespec[index] == '\\' && !IsDBCSLeadByte(filespec[index-1]))))
-       filespec[index] = '\0';
-   strcat(filespec, "/*");
+    size_t dir_len;

-   dp = (DIR *) malloc(sizeof(DIR));
+    /* skip trailing slashes in the [dir] */
+    tmp_ptr = dir + strlen(dir) - 1;
+    while (tmp_ptr > dir && (*tmp_ptr == '/' ||
+           *tmp_ptr == '\\' && !IsDBCSLeadByte(*(tmp_ptr - 1)))) {
+        tmp_ptr--;
+    }
+    if (tmp_ptr == dir && *tmp_ptr == '/') {
+        tmp_ptr--;
+    }
+
+    /* create file specification [filespec] from the [dir] */
+    dir_len = tmp_ptr - dir + 1;
+    filespec = (char *) emalloc(dir_len + 3);
+    if (filespec == NULL) {
+        /* out of memory */
+        return NULL;
+    }
+    memcpy(filespec, dir, dir_len);
+    tmp_ptr = filespec + dir_len;
+    *tmp_ptr++ = '/';
+    *tmp_ptr++ = '*';
+    *tmp_ptr = '\0';
+
+    /* init DIR structure [dp] */
+    dp = (DIR *) emalloc(sizeof(DIR));
+    if (dp == NULL) {
+        /* out of memory */
+        efree(filespec);
+        return NULL;
+    }
     dp->offset = 0;
     dp->finished = 0;
-   dp->dir = strdup(dir);
+    dp->dir = (char *) emalloc(dir_len + 2);
+    if (dp->dir == NULL) {
+        /* out of memory */
+        efree(dp);
+        efree(filespec);
+        return NULL;
+    }
+    memcpy(dp->dir, filespec, dir_len + 1);
+    dp->dir[dir_len + 1] = '\0';

+    /* init dir handle */
     if ((handle = _findfirst(filespec, &(dp->fileinfo))) < 0) {
-       if (errno == ENOENT)
+        if (errno == ENOENT) {
+            /* file specification could not be matched */
             dp->finished = 1;
-       else
+        } else {
+            /* invalid filename specification (errno == EINVAL) */
+            efree(dp->dir);
+            efree(dp);
+            efree(filespec);
             return NULL;
     }
+    }
     dp->handle = handle;
-   free(filespec);
+    efree(filespec);

     return dp;
  }

  struct dirent *readdir(DIR *dp)
  {
-   if (!dp || dp->finished)
+    size_t str_len;
+
+    if (dp == `NULL` || dp->finished) {
         return NULL;
+    }

+    /* read the next dir entry */
     if (dp->offset != 0) {
         if (_findnext(dp->handle, &(dp->fileinfo)) < 0) {
             dp->finished = 1;
@@ -64,9 +104,13 @@
     }
     dp->offset++;

-   strlcpy(dp->dent.d_name, dp->fileinfo.name, _MAX_FNAME+1);
+    /* copy dir entry to struct [dp->dent] */
+    str_len = strlen(dp->fileinfo.name);
+    str_len = str_len > _MAX_FNAME ? _MAX_FNAME : str_len;
+    memcpy(dp->dent.d_name, dp->fileinfo.name, str_len);
+    dp->dent.d_name[str_len] = '\0';
     dp->dent.d_ino = 1;
-   dp->dent.d_reclen = strlen(dp->dent.d_name);
+    dp->dent.d_reclen = str_len;
     dp->dent.d_off = dp->offset;

     return &(dp->dent);
@@ -74,11 +118,14 @@

  int readdir_r(DIR *dp, struct dirent *entry, struct dirent **result)
  {
-   if (!dp || dp->finished) {
+    size_t str_len;
+
+    if (dp == `NULL` || dp->finished) {
         *result = NULL;
         return 0;
     }

+    /* read the next dir entry */
     if (dp->offset != 0) {
         if (_findnext(dp->handle, &(dp->fileinfo)) < 0) {
             dp->finished = 1;
@@ -88,11 +135,16 @@
     }
     dp->offset++;

-   strlcpy(dp->dent.d_name, dp->fileinfo.name, _MAX_FNAME+1);
+    /* copy dir entry to struct [dp->dent] */
+    str_len = strlen(dp->fileinfo.name);
+    str_len = str_len > _MAX_FNAME ? _MAX_FNAME : str_len;
+    memcpy(dp->dent.d_name, dp->fileinfo.name, str_len);
+    dp->dent.d_name[str_len] = '\0';
     dp->dent.d_ino = 1;
-   dp->dent.d_reclen = strlen(dp->dent.d_name);
+    dp->dent.d_reclen = str_len;
     dp->dent.d_off = dp->offset;

+    /* copy struct [dp->dent] to [entry] point */
     memcpy(entry, &dp->dent, sizeof(*entry));

     *result = &dp->dent;
@@ -102,42 +154,61 @@

  int closedir(DIR *dp)
  {
-   if (!dp)
+    if (dp == NULL) {
         return 0;
+    }
+
+    /* close dir handle */
     _findclose(dp->handle);
-   if (dp->dir)
-       free(dp->dir);
-   if (dp)
-       free(dp);
+
+    /* free allocated memory */
+    if (dp->dir != NULL) {
+        efree(dp->dir);
+    }
+    if (dp != NULL) {
+        efree(dp);
+    }

     return 0;
  }

  int rewinddir(DIR *dp)
  {
-   /* Re-set to the beginning */
     char *filespec;
     long handle;
-   int index;
+    size_t dir_len;
+
+    if (dp == NULL) {
+        /* invalid pointer */
+        return -1;
+    }
+
+    /* create file specification [filespec] */
+    dir_len = strlen(dp->dir);
+    filespec = emalloc(dir_len + 3);
+    if (filespec == NULL) {
+        /* out of memory */
+        return -2;
+    }
+    memcpy(filespec, dp->dir, dir_len);
+    filespec[dir_len++] = '/';
+    filespec[dir_len++] = '*';
+    filespec[dir_len] = '\0';

+    /* close old dir handle */
     _findclose(dp->handle);

+    /* init dir handle */
     dp->offset = 0;
     dp->finished = 0;
-
-   filespec = malloc(strlen(dp->dir) + 2 + 1);
-   strcpy(filespec, dp->dir);
-   index = strlen(filespec) - 1;
-   if (index >= 0 && (filespec[index] == '/' || filespec[index] == '\\'))
-       filespec[index] = '\0';
-   strcat(filespec, "/*");
-
     if ((handle = _findfirst(filespec, &(dp->fileinfo))) < 0) {
-       if (errno == ENOENT)
+        if (errno == ENOENT) {
+            /* file specification could not be matched */
             dp->finished = 1;
         }
+    }
     dp->handle = handle;
-   free(filespec);
+    efree(filespec);

  return 0;
  }

============cut===========

-- 
Using Opera's revolutionary e-mail client: http://www.opera.com/m2/

---

## #10463 — Derick Rethans — 2004-06-15T10:47:29+00:00

URL: https://externals.io/message/10461#10463 · In reply to: #10461

> Today I checked file /win32/readdir.c
> Below you can view its source with my comments.

Just a little notice that you succesfully made it into my killfilter.
Great job!

Derick