rt_event_recv bug

发布于 2020-12-01 16:31:12 浏览：2022 订阅该版

rt_event_recv 源码中为了不出现重复代码，逻辑写的有些复杂，可以优化，用goto 或许会更清晰一些，或者适量的重复代码也可以让代码的阅读性更好。比如能正常接收时直接处理返回。而不是用if else 拖到最后。

rt_err_t rt_event_recv(rt_event_t event,rt_uint32_t   set,rt_uint8_t option,rt_int32_t timeout,rt_uint32_t *recved)
{
    struct rt_thread *thread;
    register rt_ubase_t level;
    register rt_base_t status;
    RT_DEBUG_IN_THREAD_CONTEXT;
    /* parameter check */
    RT_ASSERT(event != RT_NULL);
    RT_ASSERT(rt_object_get_type(&event->parent.parent) == RT_Object_Class_Event);
    if (set == 0)
        return -RT_ERROR;
    /* initialize status */
    status = -RT_ERROR;
    /* get current thread */
    thread = rt_thread_self();
    /* reset thread error */
    thread->error = RT_EOK;
    RT_OBJECT_HOOK_CALL(rt_object_trytake_hook, (&(event->parent.parent)));
    /* disable interrupt */
    level = rt_hw_interrupt_disable();
    /* check event set */
/*判断触发模式*/
    if (option & RT_EVENT_FLAG_AND){    
    /*判断事件触发条件是否满足或者大于 获取的逻辑与的要求 */
        if ((event->set & set) == set)
            status = RT_EOK;
    }
    else if (option & RT_EVENT_FLAG_OR){
    /*判断事件触发条件是否满足获取要求某一个条件 */
        if (event->set & set)
            status = RT_EOK;
    }
    else{
        /* either RT_EVENT_FLAG_AND or RT_EVENT_FLAG_OR should be set */
        RT_ASSERT(0);
    }
/*达到触发条件*/
    if (status == RT_EOK)
    {
        /* set received event */
    /*设置接受到的事件条件*/
        if (recved)
            *recved = (event->set & set);
        /* received event */
        /*如果条件触发是一次性的则将此条件清除*/
        if (option & RT_EVENT_FLAG_CLEAR)
            event->set &= ~set;
    }
    else if (timeout == 0) /*未达到触发条件，且 不等待*/
    {
        /* no waiting */
        thread->error = -RT_ETIMEOUT;
    }
    else /*未达到触发条件，但是超时等待*/
    {
        /* fill thread event info */
        thread->event_set  = set;
        thread->event_info = option;
        /* put thread to suspended thread list */
        /*未达到触发条件 ，且设置超时等待，则将线程挂入事件等待链表，然后设置并启动线程定时器*/
        rt_ipc_list_suspend(&(event->parent.suspend_thread),
                            thread,
                            event->parent.parent.flag);
        /* if there is a waiting timeout, active thread timer */
        if (timeout > 0)
        {
            /* reset the timeout of thread timer and start it */
            rt_timer_control(&(thread->thread_timer),
                             RT_TIMER_CTRL_SET_TIME,
                             &timeout);
            rt_timer_start(&(thread->thread_timer));
        }
        /* enable interrupt */
        rt_hw_interrupt_enable(level);
        /* do a schedule */
        /*线程进入等待，切换任务，此处会挂起*/
        rt_schedule();
        if (thread->error != RT_EOK)
        {
            /* return error */
            return thread->error;
        }
        /* received an event, disable interrupt to protect */
        level = rt_hw_interrupt_disable();
        /* set received event */
        if (recved)
            *recved = thread->event_set;
    }
    /* enable interrupt */
    rt_hw_interrupt_enable(level);
    RT_OBJECT_HOOK_CALL(rt_object_take_hook, (&(event->parent.parent)));
    return thread->error;
}
RTM_EXPORT(rt_event_recv);

修改成这样是否会更好?

rt_err_t rt_event_recv(rt_event_t event,rt_uint32_t   set,rt_uint8_t option, rt_int32_t timeout,rt_uint32_t *recved)
{
    struct rt_thread *thread;
    register rt_ubase_t level;
    register rt_base_t status;
    RT_DEBUG_IN_THREAD_CONTEXT;
    /* parameter check */
    RT_ASSERT(event != RT_NULL);
    RT_ASSERT(rt_object_get_type(&event->parent.parent) == RT_Object_Class_Event);
    if (set == 0)
        return -RT_ERROR;
    /* initialize status */
    status = -RT_ERROR;
    /* get current thread */
    thread = rt_thread_self();
    /* reset thread error */
    thread->error = RT_EOK;
    RT_OBJECT_HOOK_CALL(rt_object_trytake_hook, (&(event->parent.parent)));
    /* disable interrupt */
    level = rt_hw_interrupt_disable();
    /* check event set */
/*判断触发模式*/
    if ((option & RT_EVENT_FLAG_AND)  && ((event->set & set) == set) ||  /*判断事件触发条件是否满足获取要求某一个条件 */
        (option & RT_EVENT_FLAG_OR) && (event->set & set))        /*判断事件触发条件是否满足或者大于 获取的逻辑与的要求 */
            status = RT_EOK;
    else
          RT_ASSERT(0); /* either RT_EVENT_FLAG_AND or RT_EVENT_FLAG_OR should be set */
/*达到触发条件*/
    if (status == RT_EOK){
        thread->event_set = (event->set & set);
        goto _EXIT_OK;
    }
    if (timeout == 0) /*未达到触发条件，且 不等待*/
    {
        /* no waiting */
        thread->error = -RT_ETIMEOUT;
        goto _EXIT_ERR;
    }
       /*未达到触发条件，但是超时等待*/
    /* fill thread event info */
    thread->event_set  = set;
    thread->event_info = option;
    /* put thread to suspended thread list */
    /*未达到触发条件 ，且设置超时等待，则将线程挂入事件等待链表，然后设置并启动线程定时器*/
    rt_ipc_list_suspend(&(event->parent.suspend_thread),
                        thread,
                        event->parent.parent.flag);
    /* if there is a waiting timeout, active thread timer */
    /*这个地方 难道timeout 不是一定大于0 吗？*/
    if (timeout > 0)
    {
        /* reset the timeout of thread timer and start it */
        rt_timer_control(&(thread->thread_timer),
                         RT_TIMER_CTRL_SET_TIME,
                         &timeout);
        rt_timer_start(&(thread->thread_timer));
    }
    /* enable interrupt */
    rt_hw_interrupt_enable(level);
    /* do a schedule */
    /*线程进入等待，切换任务，此处会挂起*/
    rt_schedule();
    //此处为什么要判断这个 ，不是很明白
    if (thread->error != RT_EOK)
    {
        /* return error */
        return thread->error;
    }
    /* received an event, disable interrupt to protect */
    level = rt_hw_interrupt_disable();
_EXIT_OK:
/*设置接受到的事件条件*/
    if (recved)
        *recved = (event->set & set);
    /* received event */
    /*如果条件触发是一次性的则将此条件清除*/
    if (option & RT_EVENT_FLAG_CLEAR)
        event->set &= ~set;
_EXIT_ERR:    
    rt_hw_interrupt_enable(level);
    RT_OBJECT_HOOK_CALL(rt_object_take_hook, (&(event->parent.parent)));
    return thread->error;
}

1 个回答

mysterywolf 认证专家 2020-12-01

https://github.com/mysterywolf

c语言中，goto要慎用。尤其是这段代码已经经过10多年的检验，不可能无缘无故重新换成一种高风险的写法。
后续RTT会进行一些安全类的认证，例如MISRA2004 2012中都禁止使用goto语法。
另外问一下这个函数发现什么bug了吗？

楼兰听雨 2020-12-02

这家伙很懒，什么也没写！

这段代码的书写风格跟其他地方的有很大差别，源码也有很多地方用了goto ，我是从一个阅读者的角度触发，上面的逻辑有些绕，对新人可能不是很友好。
代码有疑惑的地方

** /*这个地方 难道timeout 不是一定大于0 吗？*/**
    if (timeout > 0)
    {
        /* reset the timeout of thread timer and start it */
        rt_timer_control(&(thread->thread_timer),
                         RT_TIMER_CTRL_SET_TIME,
                         &timeout);
        rt_timer_start(&(thread->thread_timer));
    }

还有这里

 **//此处为什么要判断这个 ，不是很明白**
    if (thread->error != RT_EOK)
    {
        /* return error */
        return thread->error;
    }

楼兰听雨 2020-12-02

这家伙很懒，什么也没写！

timeout>0 这个我是这样理解的，大于0 代表有超时限制，所以需要启动线程的定时器，而小于0 则是永久等待，不需要启用定时器。应该是这样的吧

mysterywolf 2020-12-02

https://github.com/mysterywolf

@楼兰听雨判断if (timeout > 0)是区分永久阻塞和超时阻塞永久阻塞用的是负数表示的
if (thread->error != RT_EOK) 主要是用于区分等待超时（当然还包括其他错误情况）和正确等到了事件

mysterywolf 2020-12-02

https://github.com/mysterywolf

@楼兰听雨另外rt-thread kernel 仅使用到了一处goto 没有多处

mysterywolf 2020-12-02

https://github.com/mysterywolf

@楼兰听雨在嵌入式尤其是工业控制涉及到性命攸关的设备上一定要慎用goto 虽然goto会让人的阅读体验增加，但是会直接破坏程序的逻辑，在后续维护时，稍微不注意有可能导致程序逻辑出现严重问题，goto权限太高。

请登录后再发布评论，点击登录

追加回复

rt_event_recv bug

撰写答案

关于作者

楼兰听雨

相关问题

热门标签

本月文章贡献

rt_event_recv bug

撰写答案

关于作者

楼兰听雨

相关问题

推荐文章

最新文章

热门标签

本月问答贡献

本月文章贡献