Re: [PATCH v6 1/2] jscript: implement Enumerator()

[Jacek Caban]

On 16/05/2019 08:52, Dmitry Timoshkov wrote:
> Andreas Maier  wrote:
> > +static inline HRESULT enumvar_get_next_item(EnumeratorInstance *This)
> > +{
> > - HRESULT hres;
> > - VARIANT nextitem;
> > -
> > - if (This->atend)
> > - {
> > -
> >    This->item = jsval_undefined();
> > -
> >    return S_OK;
> > - }
> > -
> > - /* dont leak pervious value */
> > - jsval_release(This->item);
> > -
> > - /* not at end ... get next item */
> > - VariantInit(&nextitem);
> > - hres = IEnumVARIANT_Next(This->enumvar, 1, &nextitem, NULL);
> > - if (hres == S_OK)
> > - {
> > -
> >    hres = variant_to_jsval(&nextitem, &This->item);
> > -
> >    if (FAILED(hres))
> > -
> >    {
> > -
> >        ERR("failed to convert jsval to variant!");
> > -
> >        This->item = jsval_undefined();
> > -
> >    }
> > - }
> > - else
> > - {
> > -
> >    This->item = jsval_undefined();
> > -
> >    This->atend = TRUE;
> > - }
> > - VariantClear(&nextitem);
> > -
> > - return S_OK;
> >
> > +}
>
> It should be possible to get rid of the 'atend' variable.

Not really, we shouldn't call Next() if we know we're at the end of 
collection. It's also needed in other places. However, there is no need 
to set item value in early return case. It should be already set by 
previous call.

Jacek