Re: [Qemu-devel] [PATCH V2] build: remove compile warning

[Markus Armbruster]

Wenchao Xia <address@hidden> writes:

> This patch simply remove "variable may be used uninitialized" warning.
>
> Signed-off-by: Wenchao Xia <address@hidden>
> ---
> V2: Address Stefan and Peter's comments, use 0 in send_msg() instead of
> initialize mhHeader.
>
>  libcacard/vscclient.c |    3 +--
>  util/iov.c            |    2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> index ac23647..7fbf1da 100644
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -641,7 +641,6 @@ main(
>      GIOChannel *channel_stdin;
>      char *qemu_host;
>      char *qemu_port;
> -    VSCMsgHeader mhHeader;
>  
>      VCardEmulOptions *command_line_options = NULL;
>  
> @@ -750,7 +749,7 @@ main(
>          .magic = VSCARD_MAGIC,
>          .capabilities = {0}
>      };
> -    send_msg(VSC_Init, mhHeader.reader_id, &init, sizeof(init));
> +    send_msg(VSC_Init, 0, &init, sizeof(init));
>  
>      g_main_loop_run(loop);
>      g_main_loop_unref(loop);

This one's actually a bit more than just a warning suppression.  The
uninitialized value gets passed to send_msg(), which prints it if
verbose > 10.

Makes no sense to me.  Comes from commit 2ac85b9; cc'ing its author for
advice.

> diff --git a/util/iov.c b/util/iov.c
> index cc6e837..b91cfb9 100644
> --- a/util/iov.c
> +++ b/util/iov.c
> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, 
> unsigned iov_cnt,
>  {
>      ssize_t total = 0;
>      ssize_t ret;
> -    size_t orig_len, tail;
> +    size_t orig_len = 0, tail;
>      unsigned niov;
>  
>      while (bytes > 0) {

Here are the uses of orig_len:

        if (tail) {
            /* second, fixup the last element, and remember the original
             * length */
            assert(niov < iov_cnt);
            assert(iov[niov].iov_len > tail);
            orig_len = iov[niov].iov_len;
            iov[niov++].iov_len = tail;
        }

        ret = do_send_recv(sockfd, iov, niov, do_send);

        /* Undo the changes above before checking for errors */
        if (tail) {
            iov[niov-1].iov_len = orig_len;
        }


gcc is too stupid to understand the control flow.  The initialization
shuts it up.

Personally, I dislike "shut up" initializations, because when you
mistakenly adds a new uninitialized use, you get the arbitrary "shut up"
value without warning.

Possible alternative:

        if (tail) {
            /* second, fixup the last element, and remember the original
             * length */
            assert(niov < iov_cnt);
            assert(iov[niov].iov_len > tail);
            orig_len = iov[niov].iov_len;
            iov[niov++].iov_len = tail;
            ret = do_send_recv(sockfd, iov, niov, do_send);
            /* Undo the changes above before checking for errors */
            iov[niov-1].iov_len = orig_len;
        } else {
            ret = do_send_recv(sockfd, iov, niov, do_send);
        }