On Thu, 24 Feb 2022 at 14:11, Hanna Reitz <hre...@redhat.com> wrote: > > 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. > > > > Some of these options are documented as always succeeding (e.g. > > CURLOPT_VERBOSE) but others have documented failure cases (e.g. > > CURLOPT_URL). For consistency we check every call, even the ones > > that theoretically cannot fail. > > > > Fixes: Coverity CID 1459336, 1459482, 1460331 > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > --- > > Changes v1->v2: > > * 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() > > --- > > block/curl.c | 92 +++++++++++++++++++++++++++++++++------------------- > > 1 file changed, 58 insertions(+), 34 deletions(-) > > Reviewed-by: Hanna Reitz <hre...@redhat.com>
For the record, I had a late thought that maybe we were setting some optional-for-us options that were only added in later versions of libcurl and accidentally relying on not checking the error code. But it turns out this isn't the case: most of the options we set are always supported, and the exceptions are: NOSIGNAL -- 7.10 onward PRIVATE -- 7.10.3 onward USERNAME, PASSWORD, PROXYUSERNAME, PROXYPASSWORD -- 7.19.1 onward, but we only set these if the user asked for them, so failing would be the right thing anyway PROTOCOLS, REDIR_PROTOCOLS -- 7.19.4 onward, guarded by a compile-time LIBCURL_VERSION_NUM check And in any case our meson.build insists on at least 7.29. (That means we could drop the LIBCURL_VERSION_NUM guards, I guess.) -- PMM