Code review comment for lp:~hipl-core/hipl/libhip

Hi,

On 12/05/12 14:00, Diego Biurrun wrote:
>>>> +int hipl_sendto(const hipl_sock_id hsock_id, const void
>>>> *const msg,
>>>>>>> + const size_t len, const int flags, +
>>>>>>> const char *const peername, uint16_t port) +{ + + if
>>>>>>> (hipl_lib_bex_feedback()) { + err =
>>>>>>> hipl_sendmsg_internal(hsock,&params, flags); + }
>>>>>>> else { + fd_set fdset; + if
>>>>>>> (hipl_hsock_ha_state(hsock) == HIP_STATE_UNASSOCIATED)
>>>>>>> { + HIP_DEBUG("Sending via hsock %d,
>>>>>>> Triggering BEX.\n", hsock->sid); + err =
>>>>>>> hipl_sendmsg_internal(hsock,&params, flags); +
>>>>>>> HIP_IFEL(err != -EWAITBEX, -1, "hipl_sendmsg_internal()
>>>>>>> failed\n"); + } + if
>>>>>>> (hipl_hsock_ha_state(hsock) == HIP_STATE_ESTABLISHED)
>>>>>>> { + HIP_DEBUG("Sending via hsock %d, HA
>>>>>>> established.\n", hsock->sid); + err =
>>>>>>> hipl_sendmsg_internal(hsock,&params, flags); + }
>>>>>>> else { + while (hipl_hsock_ha_state(hsock)
>>>>>>> != HIP_STATE_ESTABLISHED) { +
>>>>>>> FD_ZERO(&fdset); +
>>>>>>> FD_SET(hsock->sock_fd,&fdset); +
>>>>>>> HIP_DEBUG("Sending via hsock %d, Waiting BEX.\n",
>>>>>>> hsock->sid); + err =
>>>>>>> select(hsock->sock_fd + 1,&fdset, NULL, NULL, NULL); +
>>>>>>> HIP_IFEL(err< 0, -1, "select(): %s\n",
>>>>>>> strerror(errno)); + err =
>>>>>>> hipl_sendmsg_internal(hsock,&params, flags); +
>>>>>>> HIP_IFEL(err< 0&& err != -EWAITBEX&& err !=
>>>>>>> -EBEXESTABLISHED, + -1,
>>>>>>> "hipl_sendmsg_internal() failed\n"); + } +
>>>>>>> err = hipl_sendmsg_internal(hsock,&params, flags); +
>>>>>>> } + } + +out_err: + free(buf); + return err;
>>>>>>> +}
>>>>> HIP_IFEL ugliness
>>>>>
>>>
>>> I think I followed the guideline of HIP_IFEL, I use it here
>>> because I need to free memory at the end.
> I maintain that there is never any good reason to use HIP_IFEL in
> new code.

there is no strong consensus on this, this is a matter of Diego's
personal opinion especially as the matter involves memory dellocation.
So Xin, you can decide on this issue by yourself.