Re: [PATCH 1/2] oleaut32: Convert ITypeInfo implementation to use standard linked list

[Alexandre Julliard]

Andrew Eikum  writes:
> - /* search element n in list */
> - for(i=0; i < index; i++)
> - i = 0;
> - LIST_FOR_EACH_ENTRY (pTypeInfo, &This->typeinfo_list, ITypeInfoImpl, entry)
> - {
> -
>    if(i >= index)
> -
>        break;
> -
>    ++i;
> - }

It looks like an array would be more appropriate than a list.

[PATCH 1/2] oleaut32: Convert ITypeInfo implementation to use standard linked list

[Andrew Eikum]

On 12/13/2010 05:34 AM, Alexandre Julliard wrote:
> Andrew Eikum  writes:
> > - /* search element n in list */
> > - for(i=0; i<  index; i++)
> > - i = 0;
> > - LIST_FOR_EACH_ENTRY (pTypeInfo,&This->typeinfo_list, ITypeInfoImpl, entry)
> > - {
> > -
> >    if(i>= index)
> > -
> >        break;
> > -
> >    ++i;
> > - }
>
> It looks like an array would be more appropriate than a list.

An array would work, especially for the static structures that are used 
now, but these cleanup patches were made with the goal of merging the 
typelib-reading and typelib-creation interfaces in mind.  Specifically, 
implementing the typelib-creation interfaces using the current data 
structures in typelib.c.  Given the function 
ICreateTypeLib::CreateTypeInfo(), a list seems more appropriate to me.

[PATCH 1/2] oleaut32: Convert ITypeInfo implementation to use standard linked list

[Alexandre Julliard]

Andrew Eikum  writes:
> An array would work, especially for the static structures that are
> used now, but these cleanup patches were made with the goal of merging
> the typelib-reading and typelib-creation interfaces in mind.
> Specifically, implementing the typelib-creation interfaces using the
> current data structures in typelib.c.  Given the function
> ICreateTypeLib::CreateTypeInfo(), a list seems more appropriate to me.

If a list is really more appropriate for creation, then they shouldn't
share the structure. The reading API is clearly index-based, a list
isn't appropriate for that.

[PATCH 1/2] oleaut32: Convert ITypeInfo implementation to use standard linked list

[Andrew Eikum]

On 12/13/2010 10:03 AM, Alexandre Julliard wrote:
> Andrew Eikum  writes:
> > An array would work, especially for the static structures that are
> >  used now, but these cleanup patches were made with the goal of merging
> >  the typelib-reading and typelib-creation interfaces in mind.
> >  Specifically, implementing the typelib-creation interfaces using the
> >  current data structures in typelib.c.  Given the function
> >  ICreateTypeLib::CreateTypeInfo(), a list seems more appropriate to me.
>
> If a list is really more appropriate for creation, then they shouldn't
> share the structure. The reading API is clearly index-based, a list
> isn't appropriate for that.

Typelib objects on Windows support both the reading and the creation 
interfaces.  I find it unlikely that they maintain two different 
structures depending on which interface is being used, as Wine currently 
(sort of) does.  I think the question is whether the structures should 
assist reading or creation.

Would you prefer using arrays in the structures?  This would obviously 
require reallocating space for the arrays and maintaining counts in the 
implementation of the creation interfaces.  This seems more error-prone 
to me, but you have a valid point about the API being built around indexes.

[PATCH 1/2] oleaut32: Convert ITypeInfo implementation to use standard linked list

[Alexandre Julliard]

Andrew Eikum  writes:
> Typelib objects on Windows support both the reading and the creation
> interfaces.  I find it unlikely that they maintain two different
> structures depending on which interface is being used, as Wine
> currently (sort of) does.  I think the question is whether the
> structures should assist reading or creation.

Clearly it should be optimized for reading, that happens much more
frequently.