[PATCH v2] ws2_32: Drop dependencies on system getprotoby(name|number) functions

[Zebediah Figura]

On 7/14/20 10:59 AM, Alex Henrie wrote:
>  On Tue, Jul 14, 2020 at 9:41 AM Zebediah Figura <z.figura12 at gmail.com> wrote:
>>
>>> +    /* keep this list sorted by number for binary search */
>>
>> This seems to me like it should be a separate patch.
> 
> Why? I have to change the implementation of getprotobynumber anyway,
> and I don't want to cause a performance regression, so it seems that I
> might as well put in the binary search at the same time.

In that case the patch converting to binary search could be put first.

I also have some doubts that the table is large enough for binary search
to make a difference.

> 
>> If binary search is worth implementing, can we just use bsearch() to do it?
> 
> Probably yeah. I'll give that a try.
> 
>>> +    const char *names[3];
>>
>> I guess this was copied from the implementation, but surely this can
>> just be names[2]?
> 
> The NULL name at the end is used as a sentinel in the tests, the same
> as in the implementation:
> 
>             for (j = 0; protocols[i].names[0][j]; j++)
> 
>>> +    { }
>>
>> If you're using ARRAY_SIZE, does this need to be null-terminated?
> 
> Yes, it's used as a sentinel in the tests:
> 
>        if (i != ref->prot)
> 
>>> +        if (protocols[i].names[0])
>>
>> Isn't this condition always true?
> 
> Yes it is. I meant to remove this condition in v2. I will fix that in
> v3. Great catch!
> 
>>> +            ok(!ent,
>>
>> This seems like an odd place for a line break.
> 
> It's meant to mirror the format of the other ok statements in
> test_getprotobyname and test_getprotobynumber. But it's just a style
> thing; I'll delete the line break here if more people say that they
> don't like it.

I think the point of line breaks is just to avoid overly long lines, not
to put every parameter on its own line (à la Microsoft...) There's no
danger of that here.

> 
>> I guess this test structure isn't too confusing on examination, but it
>> might be clearer just to have a semi-reimplementation of
>> getprotobynumber() in the tests, to which you compare the actual data.
> 
> The test is already a semi-reimplementation of getprotobynumber: It
> has a copy of the protocol table that is identical to the
> implementation's. How is what you're suggesting different?

I mean introduce a helper function that looks up the table entry from
the number, similarly to what getprotobynumber() does.

> 
> -Alex
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200714/06beb4ac/attachment.sig>