Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:10461
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
To: internals@lists.php.net
Date: Tue, 15 Jun 2004 13:33:30 +0300
Organization: none
Content-Type: text/plain; format=flowed; delsp=yes; charset=koi8-r
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Message-ID: <opr9msx4kst8r34v@valyala>
User-Agent: Opera M2/7.50 (Win32, build 3778)
Subject: /win32/readdir.c  review
From: valyala@tut.by ("Alexander Valyalkin")

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/