Re: rsaenh: Avoid double 'va_end(args)' in case of a premature loop termination.

[Sebastian Lackner]

On 21.06.2017 10:24, Dmitry Timoshkov wrote:
> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
> ---
>  dlls/rsaenh/mpi.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/rsaenh/mpi.c b/dlls/rsaenh/mpi.c
> index 03e31023e6..dcf6ddf112 100644
> --- a/dlls/rsaenh/mpi.c
> +++ b/dlls/rsaenh/mpi.c
> @@ -2365,10 +2365,7 @@ int mp_init_multi(mp_int *mp, ...)
>                 succeeded in init-ing, then return error.
>              */
>              va_list clean_args;
> -            
> -            /* end the current list */
> -            va_end(args);
> -            
> +
>              /* now start cleaning up */            
>              cur_arg = mp;
>              va_start(clean_args, mp);
> @@ -2381,6 +2378,7 @@ int mp_init_multi(mp_int *mp, ...)
>              break;
>          }
>          n++;
> +        va_end(args);

Doesn't this change make it even worse? Whats the purpose of calling va_end()
when the enumeration isn't finished yet?
> cur_arg = va_arg(args, mp_int*);
>      }
>      va_end(args);

rsaenh: Avoid double 'va_end(args)' in case of a premature loop termination.

[Dmitry Timoshkov]

Sebastian Lackner <sebastian(a)fds-team.de> wrote:
> > diff --git a/dlls/rsaenh/mpi.c b/dlls/rsaenh/mpi.c
> > index 03e31023e6..dcf6ddf112 100644
> > --- a/dlls/rsaenh/mpi.c
> > +++ b/dlls/rsaenh/mpi.c
> > @@ -2365,10 +2365,7 @@ int mp_init_multi(mp_int *mp, ...)
> >                 succeeded in init-ing, then return error.
> >              */
> >              va_list clean_args;
> > -            
> > -            /* end the current list */
> > -            va_end(args);
> > -            
> > +
> >              /* now start cleaning up */            
> >              cur_arg = mp;
> >              va_start(clean_args, mp);
> > @@ -2381,6 +2378,7 @@ int mp_init_multi(mp_int *mp, ...)
> >              break;
> >          }
> >          n++;
> > +        va_end(args);
>
> Doesn't this change make it even worse? Whats the purpose of calling va_end()
> when the enumeration isn't finished yet?

Do you have a suggestion for a better fix?

-- 
Dmitry.

rsaenh: Avoid double 'va_end(args)' in case of a premature loop termination.

[Sebastian Lackner]

On 22.06.2017 05:40, Dmitry Timoshkov wrote:
> Sebastian Lackner <sebastian(a)fds-team.de> wrote:
> > > diff --git a/dlls/rsaenh/mpi.c b/dlls/rsaenh/mpi.c
> > > index 03e31023e6..dcf6ddf112 100644
> > > --- a/dlls/rsaenh/mpi.c
> > > +++ b/dlls/rsaenh/mpi.c
> > > @@ -2365,10 +2365,7 @@ int mp_init_multi(mp_int *mp, ...)
> > >                 succeeded in init-ing, then return error.
> > >              */
> > >              va_list clean_args;
> > > -            
> > > -            /* end the current list */
> > > -            va_end(args);
> > > -            
> > > +
> > >              /* now start cleaning up */            
> > >              cur_arg = mp;
> > >              va_start(clean_args, mp);
> > > @@ -2381,6 +2378,7 @@ int mp_init_multi(mp_int *mp, ...)
> > >              break;
> > >          }
> > >          n++;
> > > +        va_end(args);
> >
> > Doesn't this change make it even worse? Whats the purpose of calling va_end()
> > when the enumeration isn't finished yet?
>
> Do you have a suggestion for a better fix?

If I don't miss anything, just removing the duplicate va_end() call in
line 2370 should be sufficient. The "break;" will ensure it still gets
called before the function returns.

Best regards,
Sebastian

rsaenh: Avoid double 'va_end(args)' in case of a premature loop termination.

[Dmitry Timoshkov]

Sebastian Lackner <sebastian(a)fds-team.de> wrote:
> > > > diff --git a/dlls/rsaenh/mpi.c b/dlls/rsaenh/mpi.c
> > > > index 03e31023e6..dcf6ddf112 100644
> > > > --- a/dlls/rsaenh/mpi.c
> > > > +++ b/dlls/rsaenh/mpi.c
> > > > @@ -2365,10 +2365,7 @@ int mp_init_multi(mp_int *mp, ...)
> > > >                 succeeded in init-ing, then return error.
> > > >              */
> > > >              va_list clean_args;
> > > > -            
> > > > -            /* end the current list */
> > > > -            va_end(args);
> > > > -            
> > > > +
> > > >              /* now start cleaning up */            
> > > >              cur_arg = mp;
> > > >              va_start(clean_args, mp);
> > > > @@ -2381,6 +2378,7 @@ int mp_init_multi(mp_int *mp, ...)
> > > >              break;
> > > >          }
> > > >          n++;
> > > > +        va_end(args);
> > >
> > > Doesn't this change make it even worse? Whats the purpose of calling va_end()
> > > when the enumeration isn't finished yet?
> >
> > Do you have a suggestion for a better fix?
>
> If I don't miss anything, just removing the duplicate va_end() call in
> line 2370 should be sufficient. The "break;" will ensure it still gets
> called before the function returns.

Thanks, somehow an obvious solution eluded my mind.

-- 
Dmitry.