|
From: Stephen S. <rad...@gm...> - 2011-10-03 02:14:04
|
On Sun, Oct 2, 2011 at 6:23 PM, Camille Troillard <ca...@os...> wrote:
> Hi Stephen,
>
>
> Thank you for coming back to me so promptly.
>
>
> On 2 oct. 2011, at 23:40, Stephen Sinclair wrote:
>
>> Some comments,
>>
>> - There's a small typo in your sscanf line, an extra unicode
>> double-quotation mark seems to have snuck in there.
>
> Oops, thanks for noticing that.
> I later saw that the function should rather include a test to ensure that all formats were parsed:
>
> int is_dotted_ipv4_address (const char* address)
> {
> int a[4];
> return sscanf(address, "%u.%u.%u.%u", &a[0], &a[1], &a[2], &a[3]) == 4;
> }
Okay, I changed it, good catch!
>> - For SO_NOSIGPIPE, I prefer to just check for the presence of the
>> option itself instead of checking __APPLE__. (i.e. #ifdef
>> SO_NOSIGPIPE)
>
> Ah, sorry I was not aware that this could be checked.
> I guess this is it something configure provides, right?
No, it's just that it happens that SO_NOSIGPIPE is a preprocessor
define (in sys/socket.h) so it's easy enough to use the preprocessor
to check for it. Ultimately it doesn't make a difference but as much
as possible I like to stick to the convention of "check for features"
rather than "check the OS version".
> Yes, I believe that if a working address is found then it should be cached to avoid the loop each time.
>
> In the case of UDP (sendto), I am not sure to know how this can be checked before sendto is called. Maybe the caching could occur when a successful sendto is made.
>
> In the case of TCP (send), I saw that the iteration I added was a bit stupid because it actually does not change anything, so it should be removed and transferred to send.c in create_socket, around the call to connect.
Okay great, I'll do this modification and see how it works. It may be
that it's just as easy to use the same code for UDP as TCP. Although
I just realized there's actually another call to send() for the TCP
case before your loop (to send the message length), so maybe it
wouldn't have made a difference :)
Anyways I'll let you know when I apply the patch and we can test it to
make sure it doesn't introduce any problems. I'll have to find time
to test on Windows too I guess, but I don't foresee issues there. (At
least not with this patch... there seems to be some problems with
opening UDP ports on Windows 7, but that's another thing.)
thanks!
Steve
|