Re: [PATCH 3/3] regedit: Process the next file, if any, on error (v2)

[Sebastian Lackner]

On 05.07.2016 11:13, Hugh McMaster wrote:
> Changes since v1:
> * Do not leak memory when moving to the next file.
>
> Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com>
> ---
>  programs/regedit/regedit.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/programs/regedit/regedit.c b/programs/regedit/regedit.c
> index 04dd317..8a6a975 100644
> --- a/programs/regedit/regedit.c
> +++ b/programs/regedit/regedit.c
> @@ -88,7 +88,7 @@ typedef enum {
>      ACTION_ADD, ACTION_EXPORT, ACTION_DELETE
>  } REGEDIT_ACTION;
>
> -static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i)
> +static void PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i)
>  {
>      switch (action) {
>      case ACTION_ADD: {
> @@ -113,7 +113,9 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i)
>                  if (size == 0)
>                  {
>                      output_message(STRING_FILE_NOT_FOUND, filename);
> -                    exit(1);
> +                    if (realname)
> +                        HeapFree(GetProcessHeap(), 0, realname);

I haven't reviewed the whole series, but passing a NULL pointer to HeapFree()
is perfectly fine, and its better to avoid such checks.
> +                    return;
>                  }
>                  reg_file = _wfopen(realname, rb_mode);
>                  if (reg_file == NULL)
> @@ -121,7 +123,9 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i)
>                      WCHAR regedit[] = {'r','e','g','e','d','i','t',0};
>                      _wperror(regedit);
>                      output_message(STRING_CANNOT_OPEN_FILE, filename);
> -                    exit(1);
> +                    if (realname)
> +                        HeapFree(GetProcessHeap(), 0, realname);

Same here.
> +                    return;
>                  }
>                  import_registry_file(reg_file);
>                  if (realname)
> @@ -150,7 +154,6 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i)
>          exit(1);
>          break;
>      }
> -    return TRUE;
>  }
>
> BOOL ProcessCmdLine(WCHAR *cmdline)

[PATCH 3/3] regedit: Process the next file, if any, on error (v2)

[Hugh McMaster]

On Wednesday, 6 July 2016 12:09 AM, Sebastian Lackner wrote:
> On 05.07.2016 11:13, Hugh McMaster wrote:
> > @@ -113,7 +113,9 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, WCHAR **argv, int *i)
> >                   if (size == 0)
> >                   {
> >                       output_message(STRING_FILE_NOT_FOUND, filename);
> > -                    exit(1);
> > +                    if (realname)
> > +                        HeapFree(GetProcessHeap(), 0, realname);
>
> I haven't reviewed the whole series, but passing a NULL pointer to HeapFree()
> is perfectly fine, and its better to avoid such checks.

Okay, thanks for pointing that out. Alexandre has already fixed this in both places
(see http://source.winehq.org/git/wine.git/commitdiff/6ad44d7a3190d8e9a3daa9bd723...),
so no action is necessary.

Thanks for the review.