Re: [Qemu-devel] [PATCH 6/9] asn1_output-visitor.diff

[Eric Blake]

On 03/12/2013 09:09 PM, Joel Schopp wrote:
> Implement an output visitor for ASN.1 BER encoding.
> 
> Cc: Michael Tsirkin <address@hidden>
> Signed-off-by: Stefan Berger <address@hidden>
> Signed-off-by: Joel Schopp <address@hidden>
> ---

> +++ b/include/qapi/ber-output-visitor.h
> @@ -0,0 +1,28 @@
> +/*
> + * BER Output Visitor header
> + *
> + * Copyright IBM, Corp. 2011

It's 2013.

> +static unsigned int ber_encode_len(uint8_t *buffer, uint32_t buflen,
> +                                   uint64_t len, Error **errp)
> +{
> +    uint64_t mask = 0xFF00000000000000ULL;
> +    int shift =  64 - 8;
> +    int c = 0;
> +
> +    if (len <= 0x7f && buflen >= 1) {
> +        buffer[0] = len;
> +        return 1;
> +    }
> +
> +    while (mask && (mask & len) == 0) {
> +        mask >>= 8;
> +        shift -= 8;
> +    }
> +
> +    while (shift >= 0) {
> +        if (1 + c + 1 > buflen) {
> +            error_set(errp, QERR_BUFFER_OVERRUN);
> +            return 0;
> +        }
> +        buffer[1+c] = (len >> shift);

Space around binary operators.


> +
> +    if (maxnumbytes > 1) {
> +        exp_zeros = ((mask & val) == 0) ? true : false;

I'm not a fan of 'bool expr ? true : false', when just 'bool expr' will do.

> +        while (mask != 0xFF) {
> +            if (exp_zeros) {
> +                if ((mask & val) != 0) {
> +                    break;
> +                }
> +            } else {
> +                if ((mask & val) != mask) {
> +                    break;
> +                }
> +            }
> +            shift -= 8;
> +            mask >>= 8;
> +        }
> +    }
> +
> +    while (shift >= 0) {
> +        buf[2+c] = (val >> shift);

Another case of spaces around binary operator (I didn't look closely, so
you may want to scrub the entire patch for more instances than what I
saw on a casual quick read)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature