Hi Andrew,
Thank you so much for your advice! They are very useful! :-)
I have one simple question about your comments:> + struct rt_dom *sdom;
> +static void *
> +rt_alloc_domdata(const struct scheduler *ops, struct domain *dom)
> +{
> + unsigned long flags;
> + 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