On 07/03/2012 03:39 PM, Eric Blake wrote: > On 07/03/2012 03:32 PM, Eric Blake wrote: > >>> + ret = uleb128_decode_small(src + i, &count); >>> + if (ret < 0) { >> >> An nzrun should be a non-zero value; I'd write this as (ret <= 0) to >> rule out an attempt to pass a zero-length nzrun. > > Correcting myself, > > if (ret < 0 || !count) { > > At this point, I think I will just bite the bullet and post a version of > this code that incorporates my review.
Something like this (lightly tested): /* page = zrun nzrun | zrun nzrun page zrun = length nzrun = length byte... length = uleb128 encoded integer */ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen, uint8_t *dst, int dlen) { uint32_t zrun_len = 0, nzrun_len = 0; int d = 0, i = 0; long res, xor; uint8_t *nzrun_start = NULL; g_assert(!(((uintptr_t)old_buf | (uintptr_t)new_buf | slen) % sizeof(long))); while (i < slen) { /* overflow */ if (d + 2 > dlen) { return -1; } /* not aligned to sizeof(long) */ res = (slen - i) % sizeof(long); while (res && old_buf[i] == new_buf[i]) { zrun_len++; i++; res--; } if (!res) { while (i < slen && (*(long *)(old_buf + i)) == (*(long *)(new_buf + i))) { i += sizeof(long); zrun_len += sizeof(long); } /* go over the rest */ while (i < slen && old_buf[i] == new_buf[i]) { zrun_len++; i++; } } /* buffer unchanged */ if (zrun_len == slen) { return 0; } /* skip last zero run */ if (i == slen) { return d; } d += uleb128_encode_small(dst + d, zrun_len); zrun_len = 0; nzrun_start = new_buf + i; /* overflow */ if (d + 2 > dlen) { return -1; } /* not aligned to sizeof(long) */ res = (slen - i) % sizeof(long); while (res && old_buf[i] != new_buf[i]) { nzrun_len++; i++; res--; } if (!res) { /* truncation to 32-bit long okay */ long mask = 0x0101010101010101ULL; while (i < slen) { xor = *(long *)(old_buf + i) ^ *(long *)(new_buf + i); if ((xor - mask) & ~xor & (mask << 7)) { /* found the end of an nzrun within the current long */ while (old_buf[i] != new_buf[i]) { nzrun_len++; i++; } break; } else { i += sizeof(long); nzrun_len += sizeof(long); } } } d += uleb128_encode_small(dst + d, nzrun_len); /* overflow */ if (d + nzrun_len > dlen) { return -1; } memcpy(dst + d, nzrun_start, nzrun_len); d += nzrun_len; nzrun_len = 0; } return d; } int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen) { int i = 0, d = 0; int ret; uint32_t count = 0; while (i < slen) { /* zrun */ if (slen - i < 2) { return -1; } ret = uleb128_decode_small(src + i, &count); if (ret < 0 || (i && !count)) { return -1; } i += ret; d += count; /* overflow */ if (d > dlen) { return -1; } /* nzrun */ if (slen - i < 2) { return -1; } ret = uleb128_decode_small(src + i, &count); if (ret < 0 || !count) { return -1; } i += ret; /* overflow */ if (d + count > dlen || i + count > slen) { return -1; } memcpy(dst + d , src + i, count); d += count; i += count; } return d; } -- Eric Blake ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature