On Wed, Oct 13, 2010 at 12:18 AM, Christian Brunner <chb@xxxxxx> wrote:
> +static int rbd_set_snapc(rados_pool_t pool, const char *snap, RbdHeader1 *header)
> +{
> + uint32_t snap_count = header->snap_count;
> + rados_snap_t *snaps = NULL;
> + rados_snap_t seq;
> + uint32_t i;
> + uint64_t snap_names_len = header->snap_names_len;
> + int r;
> + rados_snap_t snapid = 0;
> +
> + cpu_to_le32s(&snap_count);
> + cpu_to_le64s(&snap_names_len);
It is clearer to do byteswapping immediately, rather than having the
variable take on different endianness at different times:
uint32_t snap_count = cpu_to_le32(header->snap_count);
uint64_t snap_names_len = cpu_to_le64(header->snap_names_len);
> + if (snap_count) {
> + const char *header_snap = (const char *)&header->snaps[snap_count];
> + const char *end = header_snap + snap_names_len;
snap_names_len is little-endian. This won't work on big-endian hosts.
Did you mean le64_to_cpu() instead of cpu_to_le64()?
> + snaps = qemu_malloc(sizeof(rados_snap_t) * header->snap_count);
snaps is allocated here...
> +
> + for (i=0; i < snap_count; i++) {
> + snaps[i] = (uint64_t)header->snaps[i].id;
> + cpu_to_le64s(&snaps[i]);
> +
> + if (snap && strcmp(snap, header_snap) == 0) {
> + snapid = snaps[i];
> + }
> +
> + header_snap += strlen(header_snap) + 1;
> + if (header_snap > end) {
> + error_report("bad header, snapshot list broken");
> + }
> + }
> + }
> +
> + if (snap && !snapid) {
> + error_report("snapshot not found");
> + return -ENOENT;
...but never freed here.
> + }
> + seq = header->snap_seq;
> + cpu_to_le32s((uint32_t *)&seq);
> +
> + r = rados_set_snap_context(pool, seq, snaps, snap_count);
> +
> + rados_set_snap(pool, snapid);
> +
> + qemu_free(snaps);
> +
> + return r;
> +}
Stefan
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
