Re: [PATCH RFC v2 1/4] xen: add real time scheduler rt

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2783 bytes --]

On 07/08/14 12:26, Meng Xu wrote:
> Hi Andrew,
>
> ​Thank you so much for your advice! They are very useful! :-)
>
> I have one simple question about your comments:
> ​
>
>>
>>     > +static void *
>>     > +rt_alloc_domdata(const struct scheduler *ops, struct domain *dom)
>>     > +{
>>     > +    unsigned long flags;
>>     > +    struct rt_dom *sdom;
>>
>>     > +    struct rt_private * prv = RT_PRIV(ops);
>>     > +
>>     > +    printtime();
>>     > +    printk("dom=%d\n", dom->domain_id);
>>     > +
>>     > +    sdom = xzalloc(struct rt_dom);
>>     > +    if ( sdom == NULL )
>>     > +    {
>>     > +        printk("%s, xzalloc failed\n", __func__);
>>     > +        return NULL;
>>     > +    }
>>     > +
>>     > +    INIT_LIST_HEAD(&sdom->vcpu);
>>     > +    INIT_LIST_HEAD(&sdom->sdom_elem);
>>     > +    sdom->dom = dom;
>>     > +
>>     > +    /* spinlock here to insert the dom */
>>     > +    spin_lock_irqsave(&prv->lock, flags);
>>     > +    list_add_tail(&sdom->sdom_elem, &(prv->sdom));
>>     > +    spin_unlock_irqrestore(&prv->lock, flags);
>>     > +
>>     > +    return (void *)sdom;
>>
>>         Bogus cast.
>>
>>
>>     ​I think we have to cast it to void * ​because the definition of
>>     this function asks the return type to be void *. In addition, the
>>     credit2 scheduler also did the same cast in  this _alloc_domdata
>>     function. So I guess this should be fine?
>>
>
>     In C, all pointers are implicitly castable to/from void*.  This is
>     not the case in C++.  (which suggests to me that George learnt C++
>     before C, or is at least more familiar with C++?)
>
>     The act of using type punning means that you are trying to do
>     something which the C type system doesn't like.  This in turn
>     requires more thought from people reading the code to work out
>     whether it is actually safe or not.  As a result, we strive for as
>     few type overrides as possible.
>
>     Please don't fall into the trap of assuming the credit2 code, or
>     indeed any of the other code in Xen, is perfect.  None of it is,
>     although most of it is good.
>
>
> ​I can change the return (void *)sdom to return sdom.
> ​But I'm not sure what else should I modify?
> In sched-if.h,  this function is ​"void *       (*alloc_domdata)
>  (const struct scheduler *, struct domain *);" I think I should not
> change this declaration, otherwise, the modification will affect the
> compilation of other schedulers.
> ​​
> ​Thank you so much!
>
> Best,
>
> Meng​

return sdom; is perfectly fine with the existing function prototype.  It
is an implicit cast of a pointer to void*, which is perfectly legal in C.

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 7654 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel