Re: [Xen-devel] [PATCH v4 1/9] xen/arm: Implement hvm save and restore

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-10-08 at 06:30 +0000, Jaeyong Yoo wrote:

> >> +static void vgic_irq_rank_save(struct vgic_rank *ext,
> >> +                               struct vgic_irq_rank *rank)
> >> +{
> >> +    spin_lock(&rank->lock);
> >> +    /* Some of VGIC registers are not used yet, it is for a future usage 
> >> */
> >> +    /* IENABLE, IACTIVE, IPEND,  PENDSGI registers */
> >> +    ext->ienable = rank->ienable;
> >> +    ext->iactive = rank->iactive;
> >> +    ext->ipend = rank->ipend;
> >> +    ext->pendsgi = rank->pendsgi;
> >> +    /* ICFG */
> >> +    ext->icfg[0] = rank->icfg[0];
> >> +    ext->icfg[1] = rank->icfg[1];
> >Can you use memcpy?

Why?

> OK.

If ext->icfg[0] rank->icfg[0] are the same type then I think using
assignment is better, since it is more type safe and it will effectively
get turned into a memcpy by the compiler anyway.

If they are different types then using memcpy just runs the risk that
one of them will silently change and break things.

With that in mind can these not be proper assignments too:

> >> +    /* IPRIORITY */
> >> +    memcpy(ext->ipriority, rank->ipriority, sizeof(rank->ipriority));
> >> +    /* ITARGETS */
> >> +    memcpy(ext->itargets, rank->itargets, sizeof(rank->itargets));

Or if not then copy field by field would actually be preferable IMHO. A
helper macro can make it a bit tidier sometimes:
        #define C(F) ext->#F = rank->#F

then
        C(itargets.foo);
        C(itergets.bar);
        #undef C
etc.

Ian.


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