Re: [Tarantool-patches] [PATCH 1/2] swim: add SO_BROADCAST option

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

>>>> diff --git a/src/lib/swim/swim_io.c b/src/lib/swim/swim_io.c
>>>> index c8558c43e..45df36ba4 100644
>>>> --- a/src/lib/swim/swim_io.c
>>>> +++ b/src/lib/swim/swim_io.c
>>>> @@ -512,8 +512,23 @@ static inline void
>>>>  swim_complete_send(struct swim_scheduler *scheduler, struct swim_task *task,
>>>>  		   ssize_t size)
>>>>  {
>>>> -	if (size < 0)
>>>> -		diag_log();
>>>> +	if (size < 0) {
>>>> +		bool is_critical = false;
>>>> +#if TARGET_OS_DARWIN
>>>> +		/*
>>>> +		 * On Mac this error happens regularly if SWIM is bound to
>>>> +		 * the localhost and tries to broadcast out of the machine. This
>>>> +		 * is not critical, because will happen in the tests a lot, and
>>>> +		 * in prod it simply should not bind to localhost if there are
>>>> +		 * multiple machines in the cluster. Besides, Mac as a platform
>>>> +		 * is not supposed to be used in prod.
>>>> +		 */
>>>> +		struct error *last = diag_last_error(diag_get());
>>>> +		is_critical = (last->saved_errno == EADDRNOTAVAIL);
>>>> +#endif
>>>> +		if (is_critical)
>>>> +			diag_log();
>>>> +	}
>>>>  	if (task->complete != NULL)
>>>>  		task->complete(task, scheduler, size);
>>>
>>> Vlad, I don't understand. For non-mac users this @is_critical will
>>> always be false, maybe we better move the whoe branch to TARGET_OS_DARWIN
>>> then? Ie
>>>
>>> 	if (size < 0) {
>>> 	#if TARGET_OS_DARWIN
>>> 		if (is_critical)
>>> 			diag_log();
>>> 	#endif
>>> 	}
>>>
>>> or you made it this way to escape unused parameter compiler warning?
>>
>> I added the variable on all platforms on purpose. Because if the code would
>> be done the way you propose above, non-Mac platforms won't log anything.
> 
> The do not log anything with the patch. Lets drop TARGET_OS_DARWIN path
> from your patch and we will have
> 
> swim_complete_send(struct swim_scheduler *scheduler, struct swim_task *task,
>                   ssize_t size)
> {
> 	if (size < 0) {
>                bool is_critical = false;
>                if (is_critical)
>                        diag_log();
> 	}
> }
> 
> The is_critical is always false, or there is a typo in the code above.

Oh shit, indeed :)