Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
lib: simple_file_selector(): simplify the error path to confuse covsc…
Browse files Browse the repository at this point in the history
…an less.

Because they don't believe code should be defensive against future
changes, covscan believes:

520 out_free:
521        FreePool(dmp);
   CID 182824 (#1 of 1): Dereference before null check
   (REVERSE_INULL)check_after_deref: Null-checking entries suggests that
   it may be null, but it has already been dereferenced on all paths
   leading to the check.
522        if (entries) {
523                free_entries(entries, count);
524                FreePool(entries);
525        }
526 out_free_name:
527        FreePool(name);
528}

Which is technically correct, but still kind of dumb.  So this patch
combines the two error out paths into just being out_free, so that the
first path there is before entries is allocated.  (It also initializes
dmp to NULL and checks that before freeing it.)

I also Lindent-ed that function.

Signed-off-by: Peter Jones <[email protected]>
vathpela committed Oct 19, 2017

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent b730a5b commit 3d2fdc0
Showing 1 changed file with 13 additions and 14 deletions.
27 changes: 13 additions & 14 deletions lib/simple_file.c
Original file line number Diff line number Diff line change
@@ -411,12 +411,12 @@ free_entries(CHAR16 **entries, int count)
}

void
simple_file_selector(EFI_HANDLE *im, CHAR16 **title, CHAR16 *name,
CHAR16 *filter, CHAR16 **result)
simple_file_selector(EFI_HANDLE * im, CHAR16 ** title, CHAR16 * name,
CHAR16 * filter, CHAR16 ** result)
{
EFI_STATUS status;
CHAR16 **entries = NULL;
EFI_FILE_INFO *dmp;
EFI_FILE_INFO *dmp = NULL;
int count, select, len;
CHAR16 *newname, *selected;

@@ -436,18 +436,17 @@ simple_file_selector(EFI_HANDLE *im, CHAR16 **title, CHAR16 *name,
*im = h;
}

newname = AllocatePool((StrLen(name) + 1)*sizeof(CHAR16));
newname = AllocatePool((StrLen(name) + 1) * sizeof(CHAR16));
if (!newname)
return;

StrCpy(newname, name);
name = newname;

redo:
redo:
status = simple_dir_filter(*im, name, filter, &entries, &count, &dmp);

if (status != EFI_SUCCESS)
goto out_free_name;
goto out_free;

select = console_select(title, entries, 0);
if (select < 0)
@@ -471,7 +470,6 @@ simple_file_selector(EFI_HANDLE *im, CHAR16 **title, CHAR16 *name,

i = StrLen(name) - 1;


for (i = StrLen(name); i > 0; --i) {
if (name[i] == '\\')
break;
@@ -489,11 +487,12 @@ simple_file_selector(EFI_HANDLE *im, CHAR16 **title, CHAR16 *name,
goto redo;
}
}
newname = AllocatePool((StrLen(name) + len + 2)*sizeof(CHAR16));
newname =
AllocatePool((StrLen(name) + len + 2) * sizeof(CHAR16));
if (!newname)
goto out_free;
StrCpy(newname, name);

if (name[StrLen(name) - 1] != '\\')
StrCat(newname, L"\\");
StrCat(newname, selected);
@@ -509,20 +508,20 @@ simple_file_selector(EFI_HANDLE *im, CHAR16 **title, CHAR16 *name,

goto redo;
}
*result = AllocatePool((StrLen(name) + len + 2)*sizeof(CHAR16));
*result = AllocatePool((StrLen(name) + len + 2) * sizeof(CHAR16));
if (*result) {
StrCpy(*result, name);
if (name[StrLen(name) - 1] != '\\')
StrCat(*result, L"\\");
StrCat(*result, selected);
}

out_free:
FreePool(dmp);
out_free:
if (dmp)
FreePool(dmp);
if (entries) {
free_entries(entries, count);
FreePool(entries);
}
out_free_name:
FreePool(name);
}

0 comments on commit 3d2fdc0

Please sign in to comment.