Re: [Xen-devel] [PATCH 5 of 6] x86: fix memset(ptr, 0, sizeof ptr)

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.04.12 at 14:07, Tim Deegan <tim@xxxxxxx> wrote:
> # HG changeset patch
> # User Tim Deegan <tim@xxxxxxx>
> # Date 1333626954 -3600
> # Node ID 5101e5ed24732919076d5285e55c7b53032749c2
> # Parent  dfe9453c066f3fbfa09c847d7ec381cdc0da0f99
> x86: fix memset(ptr, 0, sizeof ptr).
> 
> Signed-off-by: Tim Deegan <tim@xxxxxxx>
> 
> diff -r dfe9453c066f -r 5101e5ed2473 xen/arch/x86/cpu/mcheck/amd_f10.c
> --- a/xen/arch/x86/cpu/mcheck/amd_f10.c       Thu Apr 05 12:55:54 2012 +0100
> +++ b/xen/arch/x86/cpu/mcheck/amd_f10.c       Thu Apr 05 12:55:54 2012 +0100
> @@ -73,9 +73,9 @@ amd_f10_handler(struct mc_info *mi, uint
>               return NULL;
>       }
>  
> -     memset(mc_ext, 0, sizeof(mc_ext));
> +     memset(mc_ext, 0, sizeof(*mc_ext));
>       mc_ext->common.type = MC_TYPE_EXTENDED;
> -     mc_ext->common.size = sizeof(mc_ext);
> +     mc_ext->common.size = sizeof(*mc_ext);
>       mc_ext->mc_msrs = 3;
>  
>       mc_ext->mc_msr[0].reg = MSR_F10_MC4_MISC1;
> diff -r dfe9453c066f -r 5101e5ed2473 xen/arch/x86/mm/p2m.c
> --- a/xen/arch/x86/mm/p2m.c   Thu Apr 05 12:55:54 2012 +0100
> +++ b/xen/arch/x86/mm/p2m.c   Thu Apr 05 12:55:54 2012 +0100
> @@ -1236,7 +1236,7 @@ bool_t p2m_mem_access_check(unsigned lon
>      if ( req )
>      {
>          *req_ptr = req;
> -        memset(req, 0, sizeof(req));
> +        memset(req, 0, sizeof (mem_event_request_t));

Why not

        memset(req, 0, sizeof (*req));

? If req's type changes, you still want to clear all of it (and nothing
more).

Oh, wait - this gets xmalloc()-ed immediately before that if(). Mind
simply switching that one to xzalloc(), and deleting the memset()
altogether? (I wonder how many other xmalloc()/memset() pairs
went in again since the introduction of xzalloc() - one of the
purposes of this was to avoid this particular common mistake.)

Apart from that
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

>          req->reason = MEM_EVENT_REASON_VIOLATION;
>  
>          /* Pause the current VCPU */
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 




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