mbox series

[v2,0/2] block/curl: check error return from curl_easy_setopt()

Message ID 20220222152341.850419-1-peter.maydell@linaro.org
Headers show
Series block/curl: check error return from curl_easy_setopt() | expand

Message

Peter Maydell Feb. 22, 2022, 3:23 p.m. UTC 
Coverity points out that we aren't checking the return value
from curl_easy_setopt() for any of the calls to it we make
in block/curl.c.

Tested with 'make check' and with some basic smoke test command lines
suggested by Dan:

 qemu-img info https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2
 qemu-img info --image-opts driver=qcow2,file.driver=https,file.url=https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2

Changes v1->v2:
 * new patch 1 which fixes a missing "set the error string" for
   when curl_init_state() returns failure, since we're about to
   add more cases when that function can fail
 * set the error string in the failure path for the direct setopt
   calls in curl_open()
 * fix the failure path in curl_setup_preadv() by putting
   the curl_easy_setopt() call in the same if() condition
   as the existing curl_multi_add_handle()

thanks
-- PMM

Peter Maydell (2):
  block/curl.c: Set error message string if curl_init_state() fails
  block/curl.c: Check error return from curl_easy_setopt()

 block/curl.c | 94 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 34 deletions(-)

Comments

Hanna Reitz Feb. 24, 2022, 2:13 p.m. UTC | #1 
On 22.02.22 16:23, Peter Maydell wrote:
> Coverity points out that we aren't checking the return value
> from curl_easy_setopt() for any of the calls to it we make
> in block/curl.c.
>
> Tested with 'make check' and with some basic smoke test command lines
> suggested by Dan:
>
>   qemu-img info https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2
>   qemu-img info --image-opts driver=qcow2,file.driver=https,file.url=https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2
>
> Changes v1->v2:
>   * new patch 1 which fixes a missing "set the error string" for
>     when curl_init_state() returns failure, since we're about to
>     add more cases when that function can fail
>   * set the error string in the failure path for the direct setopt
>     calls in curl_open()
>   * fix the failure path in curl_setup_preadv() by putting
>     the curl_easy_setopt() call in the same if() condition
>     as the existing curl_multi_add_handle()

Thanks, applied to my block branch:

https://gitlab.com/hreitz/qemu/-/commits/block

Hanna