On 10 March 2015 at 16:11, Maxim Uvarov <[email protected]> wrote:
> On 03/10/15 18:08, Ola Liljedahl wrote:
>>
>> On 10 March 2015 at 15:59, Maxim Uvarov <[email protected]> wrote:
>>>
>>> On 03/10/15 17:43, Ola Liljedahl wrote:
>>>>
>>>> Ensure that the timeout user_ptr and timer fields are set when the
>>>> corresponding timer is immediately cancelled.
>>>> https://bugs.linaro.org/show_bug.cgi?id=1313
>>>>
>>>> Signed-off-by: Ola Liljedahl <[email protected]>
>>>> ---
>>>> (This document/code contribution attached is provided under the terms of
>>>> agreement LES-LTM-21309)
>>>>
>>>> Passes odp_timer validation with the new odp_timer_cancel() test from
>>>> Petri.
>>>>
>>>> platform/linux-generic/odp_timer.c | 18 ++++++++++++++----
>>>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/platform/linux-generic/odp_timer.c
>>>> b/platform/linux-generic/odp_timer.c
>>>> index 61a02b6..6b48d2e 100644
>>>> --- a/platform/linux-generic/odp_timer.c
>>>> +++ b/platform/linux-generic/odp_timer.c
>>>> @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
>>>> #endif
>>>> } else {
>>>> /* We have a new timeout buffer which replaces any old
>>>> one
>>>> */
>>>> + /* Fill in header fields if timeout event */
>>>> + if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
>>>> + /* Convert from buffer to timeout hdr */
>>>> + odp_timeout_hdr_t *tmo_hdr =
>>>> + timeout_hdr_from_buf(*tmo_buf);
>>>
>>> as we discussed earlier this 2 definitions of tmo_hdr can be one on top
>>> of
>>> timer_expire().
>>> int succ can be int ret and also defined on top.
>>
>> Yes they can but that would be a different coding style. It is OK to
>> declare variables at beginning of a block, even Linux does it (see
>> Petri's example). This has been OK since the beginning of time (the
>> start of the Epoch, Jan. 1st 1970).
>
>
> My point here is if you use one variable 2 times in one function, than most
> probably you need define it only once.
> Do you agree?
No.
There is no intrinsic value in having as few variable declarations as
possible. If the scope becomes larger, this could be confusing to the
programmers that try to understand the code. A larger scope means that
the variables are valid longer (from a compiler perspective) even if
the variables are not really valid from a runtime perspective.
If I only need to use a variable in a certain block (because the
corresponding value is only alive there), I want to declare it in that
block and have it go out of scope when the block ends. Possibly
another block in the same function needs a variable of the same type
for some short-lived purpose. Then I will declare that variable in
that block and not try to combine in with an unrelated variable in
another block just because they are of the same type and used for
similar purposes.
>
> Maxim.
>
>>
>>> Maxim.
>>>
>>>> + tmo_hdr->timer = tp_idx_to_handle(tp, idx);
>>>> + tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
>>>> + /* expiration field filled in when timer expires
>>>> */
>>>> + }
>>>> + /* Else ignore buffers of other types */
>>>> odp_buffer_t old_buf = ODP_BUFFER_INVALID;
>>>> #ifdef ODP_ATOMIC_U128
>>>> tick_buf_t new, old;
>>>> @@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp,
>>>> uint32_t idx, uint64_t tick)
>>>> _odp_atomic_flag_clear(IDX2LOCK(idx));
>>>> #endif
>>>> if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
>>>> - /* Fill in metadata fields in system timeout buffer */
>>>> + /* Fill in expiration tick if timeout event */
>>>> if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
>>>> /* Convert from buffer to timeout hdr */
>>>> odp_timeout_hdr_t *tmo_hdr =
>>>> timeout_hdr_from_buf(tmo_buf);
>>>> - tmo_hdr->timer = tp_idx_to_handle(tp, idx);
>>>> tmo_hdr->expiration = exp_tck;
>>>> - tmo_hdr->user_ptr = tim->user_ptr;
>>>> + /* timer and user_ptr fields filled in when
>>>> timer
>>>> + * was set */
>>>> }
>>>> - /* Else ignore buffers of other types */
>>>> + /* Else ignore events of other types */
>>>> /* Post the timeout to the destination queue */
>>>> int rc = odp_queue_enq(tim->queue,
>>>> odp_buffer_to_event(tmo_buf));
>>>
>>>
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> [email protected]
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
