-
Notifications
You must be signed in to change notification settings - Fork 333
dirent cleanup #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dirent cleanup #47
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test
DIR * opendir(const char *name) | ||
{ | ||
/* | ||
* Open a directory stream on NAME. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the NAME is capitalized?
if (wcscmp(c_file.name, L".") == 0 || wcscmp(c_file.name, L"..") == 0 ) | ||
continue; | ||
|
||
if ((pdirentry = malloc(sizeof(struct dirent))) == NULL || | ||
(tmp = utf16_to_utf8(c_file.name)) == NULL) { | ||
if (pdirentry) | ||
free(pdirentry); | ||
errno = ENOMEM; | ||
return NULL; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for line 134: strncpy(pdirentry->d_name, tmp, strlen(tmp) + 1);
Should it be strncpy(pdirentry->d_name, tmp, sizeof(pdirentry->d_name)); in case tmp is longer than size of d_name?
or need to make sure the length of pdirentry->d_name has enough space for tmp.
swprintf_s(searchstr, MAX_PATH, L"%s\\*.*", wname); | ||
/* add *.* for Windows _findfirst() search pattern */ | ||
if (swprintf(searchstr, MAX_PATH, L"%s\\*.*", wname) == -1) { | ||
/* breached MAX_PATH */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swprintfreturns the number of wide characters stored in buffer, not counting the terminating null wide character. so this comment "breached MAX_PATH" is misleading?
If we are unable to write to searchstr then we will be inside if loop.
int | ||
closedir(DIR *dirp) { | ||
if ( dirp && dirp->hFile) { | ||
_findclose(dirp->hFile); | ||
dirp->hFile = 0; | ||
free (dirp); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to free(dirp) irrespective of dirp->hFile.. so plz move this out of if loop..
/* Read a directory entry from DIRP. */ | ||
struct dirent* | ||
readdir(void *avp) { | ||
struct dirent *pdirentry = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plz update the code as I have modified this to static to avoid the memory leaks..
else if (_wfindnext(dirp->hFile, &c_file) != 0) | ||
return NULL; | ||
|
||
dirp->first = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will execute everytime even though it is 0 so better to move it inside if loop at line 116.
No description provided.