[1/2] ole32: Add helper for string table memory freeing
Frédéric Delanoy
frederic.delanoy at gmail.com
Mon Nov 26 05:34:21 CST 2012
On Mon, Nov 26, 2012 at 8:54 AM, Nikolay Sivov <bunglehead at gmail.com> wrote:
> On 11/26/2012 01:01, Frédéric Delanoy wrote:
>>
>> ---
>> dlls/ole32/filemoniker.c | 44
>> ++++++++++++++++++--------------------------
>> 1 file changed, 18 insertions(+), 26 deletions(-)
>>
>> diff --git a/dlls/ole32/filemoniker.c b/dlls/ole32/filemoniker.c
>> index b3a5cc2..688f6b8 100644
>> --- a/dlls/ole32/filemoniker.c
>> +++ b/dlls/ole32/filemoniker.c
>> @@ -657,6 +657,16 @@ FileMonikerImpl_Reduce(IMoniker* iface, IBindCtx*
>> pbc, DWORD dwReduceHowFar,
>> return MK_S_REDUCED_TO_SELF;
>> }
>> +static void
>> +FileMonikerImpl_FreeStringTable(LPOLESTR* stringTable)
>> +{
>> + int i;
>> +
>> + for (i=0; stringTable[i]!=NULL; i++)
>> + CoTaskMemFree(stringTable[i]);
>> + CoTaskMemFree(stringTable);
>> +}
>> +
>
> Could you please adjust a naming for that to something like
> free_stringtable() cause it doesn't really access file moniker in this
> helper.
OK
>Also stringTable[i]!=NULL could be shortened to stringTable[i].
I know but it's mostly a matter of style. I prefer the '!= NULL'
notation when accessing it "array-style", and any sane compiler would
optimize it anyway.
>> CoTaskMemFree(str1);
>> CoTaskMemFree(str2);
>> @@ -1004,17 +1010,9 @@ FileMonikerImpl_CommonPrefixWith(IMoniker*
>> iface,IMoniker* pmkOther,IMoniker** p
>> {
>> for(i=0;i<sameIdx;i++)
>> strcatW(commonPath,stringTable1[i]);
>> -
>> - for(i=0;i<nb1;i++)
>> - CoTaskMemFree(stringTable1[i]);
>> -
>> - CoTaskMemFree(stringTable1);
>> -
>> - for(i=0;i<nb2;i++)
>> - CoTaskMemFree(stringTable2[i]);
>> -
>> - CoTaskMemFree(stringTable2);
>> -
>> +
>> + FileMonikerImpl_FreeStringTable(stringTable1);
>> + FileMonikerImpl_FreeStringTable(stringTable2);
>
> You sure it's an equivalent change? I mean regarding index boundaries above.
It's equivalent.
> Also while you're at it FileMonikerImpl_DecomposePath() needs to be
> properly fixed - it returns HRESULT, not it. And surrounding code relies on
> FAILED() mask to get this index boundary from return value, this is really
> broken.
Yes, but that probably warrants a patch of its own, to keep this one
as self-contained/atomic as possible.
I'll look into that
Frédéric
More information about the wine-devel
mailing list