On 12 March 2018 at 15:21, Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> wrote: > Minimal realization: only one extent in server answer is supported. > Flag NBD_CMD_FLAG_REQ_ONE is used to force this behavior.
Hi; Coverity complains about a possible use of uninitialized variables in this function (CID 1390607, 1390611): > +static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, > + const char *export, > + const char *context, > + uint32_t *context_id, > + Error **errp) > +{ > + int ret; > + NBDOptionReply reply; > + uint32_t received_id; > + bool received; We declare received_id and received without initializing them... > + uint32_t export_len = strlen(export); > + uint32_t context_len = strlen(context); > + uint32_t data_len = sizeof(export_len) + export_len + > + sizeof(uint32_t) + /* number of queries */ > + sizeof(context_len) + context_len; > + char *data = g_malloc(data_len); > + char *p = data; > + > + stl_be_p(p, export_len); > + memcpy(p += sizeof(export_len), export, export_len); > + stl_be_p(p += export_len, 1); > + stl_be_p(p += sizeof(uint32_t), context_len); > + memcpy(p += sizeof(context_len), context, context_len); > + > + ret = nbd_send_option_request(ioc, NBD_OPT_SET_META_CONTEXT, data_len, > data, > + errp); > + g_free(data); > + if (ret < 0) { > + return ret; > + } > + > + if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply, > + errp) < 0) > + { > + return -1; > + } > + > + ret = nbd_handle_reply_err(ioc, &reply, errp); > + if (ret <= 0) { > + return ret; > + } > + > + if (reply.type == NBD_REP_META_CONTEXT) { ...and if we don't take this code path we don't ever initialize received... > + char *name; > + size_t len; > + > + if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) { > + return -EIO; > + } > + be32_to_cpus(&received_id); > + > + len = reply.length - sizeof(received_id); > + name = g_malloc(len + 1); > + if (nbd_read(ioc, name, len, errp) < 0) { > + g_free(name); > + return -EIO; > + } > + name[len] = '\0'; > + if (strcmp(context, name)) { > + error_setg(errp, "Failed to negotiate meta context '%s', server " > + "answered with different context '%s'", context, > + name); > + g_free(name); > + return -1; > + } > + g_free(name); > + > + received = true; > + > + /* receive NBD_REP_ACK */ > + if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply, > + errp) < 0) > + { > + return -1; > + } > + > + ret = nbd_handle_reply_err(ioc, &reply, errp); > + if (ret <= 0) { > + return ret; > + } > + } > + > + if (reply.type != NBD_REP_ACK) { > + error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x", > + reply.type, NBD_REP_ACK); > + return -1; > + } > + > + if (received) { > + *context_id = received_id; ...so we might use both received and received_id uninitialized here. > + return 1; > + } > + > + return 0; > +} My guess is that the correct fix is just to initialize received with "bool received = false;". Coverity should then be able to figure out that we don't touch received_id unless we initialized it. thanks -- PMM