/win32/readdir.c review

21 years ago by Alexander Valyalkin — view source — reply

unread

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/

21 years ago by Alexander Valyalkin — view source — reply

unread

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/

21 years ago by Derick Rethans — view source — reply

unread

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