Re: [Xen-devel] [PATCH v3 1/2] xen/arm: Start to implement an ARM decoder instruction

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-07-31 at 17:39 +0100, Julien Grall wrote:
> On 07/31/2013 05:26 PM, Ian Campbell wrote:
> > On Wed, 2013-07-31 at 17:18 +0100, Julien Grall wrote:
> > 
> > 
> >>>> +        switch ( opB & 0x3 )
> >>>> +        {
> >>>> +        case 0:
> >>>> +            dabt->size = 2;
> >>>> +            break;
> >>>> +        case 1:
> >>>> +            dabt->size = 1;
> >>>
> >>> ->sign is uninitialised for these two cases?
> >>>
> >>> Actually, for many of them I think?
> >>
> >> I plan to zeroed the ISS field (ie sign, reg...) by default. See TODO in
> >> decode_instruction. Do I still need to set sign to 0?
> > 
> > Well, you need to at least do one or the other!
> > 
> > Even if you are zeroing the iss be default I think it would be useful
> > from a documentation/clarity perspective to set it explicitly to zero.
> 
> There is only few functions which deal with signed-register. So if we
> add a line "dabt->sign = 0" it's less clear.

Even if the function doesn't deal with a signed register ever the
consumer of the decoded hsr doesn't know that and needs to know not to
sign extend, so it needs to be cleared explicitly somewhere.

>  What about a helper?
> 
> update_dabt(struct hsr_dabt *dabt, uint16_t reg, uint8_t size,
>             bool_t sign)




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