Skip to content

Commit

Permalink
del: use findnext() to avoid leaks [fixes #84]
Browse files Browse the repository at this point in the history
It appears findnext() needs to be called until the search is
completed, and only in that case djgpp closes the findfirst handle.
Call findnext() before removing the file to stay on a safe side.
Perhaps glob() would be a better solution.

Using only findfirst() was fine before LFNs were enabled, because
the SFN's findfirst() doesn't need to close searches. But with
LFNs this eventually overflows the buffer.
  • Loading branch information
stsp committed Nov 9, 2023
1 parent ef0fdeb commit 346193d
Showing 1 changed file with 9 additions and 7 deletions.
16 changes: 9 additions & 7 deletions src/command.c
Original file line number Diff line number Diff line change
Expand Up @@ -2054,10 +2054,12 @@ static void perform_date(const char *arg)

static void perform_delete(const char *arg)
{
long fhandle;
finddata_t ff;
char filespec[MAXPATH] = "";
char full_filespec[MAXPATH] = "";
char drive[MAXDRIVE], dir[MAXDIR];
int done = 0;

while (*arg != '\0')
{
Expand All @@ -2084,18 +2086,17 @@ static void perform_delete(const char *arg)
return;
}

if (findfirst_f(filespec, &ff, 0, NULL) != 0)
{
printf("File(s) not found - %s\n", filespec); // informational msg; not an error
return;
}

_fixpath(filespec, full_filespec);
fnsplit(full_filespec, drive, dir, NULL, NULL);
conv_unix_path_to_ms_dos(drive);
conv_unix_path_to_ms_dos(dir);

while (findfirst_f(full_filespec, &ff, 0, NULL) == 0)
if (findfirst_f(full_filespec, &ff, 0, &fhandle) != 0)
{
printf("File(s) not found - %s\n", filespec); // informational msg; not an error
return;
}
while (!done)
{
char individual_filespec[MAXPATH];

Expand All @@ -2104,6 +2105,7 @@ static void perform_delete(const char *arg)
strcat(individual_filespec, dir);
strcat(individual_filespec, FINDDATA_T_FILENAME(ff));

done = (findnext_f(&ff, fhandle) != 0);
if (remove(individual_filespec) == 0)
printf("%s erased\n", individual_filespec);
else
Expand Down

3 comments on commit 346193d

@andrewbird
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't dos delete respect wildcards since V3?

@stsp
Copy link
Member Author

@stsp stsp commented on 346193d Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOS maybe, but not remove()/unlink().
For now I don't want to circumvent the
posix APIs in comcom32, and in this
particular case its not even worth a try.

@stsp
Copy link
Member Author

@stsp stsp commented on 346193d Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ansi/stdio/remove.c:

  if (use_lfn)
  {
    r.x.flags = 1;              /* Always set CF before calling a 0x71NN functio
n. */
    r.h.al = r.h.ah;
    r.h.ah = 0x71;
    r.x.si = 0;                 /* No Wildcards */

Please sign in to comment.