[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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!



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.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.