On 11/14/2017 12:01 PM, Max Reitz wrote: > bdrv_reopen_prepare() assumes that all BDS options are strings, which is > not necessarily correct. This series introduces a new qobject_is_equal() > function which can be used to test whether any options have changed, > independently of their type. > > > v7: > - Patch 6: Fix a clang warning: > tests/check-qobject.c:39:24: error: passing an object that undergoes > default argument promotion to > 'va_start' has undefined behavior > TIL: You cannot use va_start(ap, foo) if @foo is a bool. An int > works, however.
I knew that va_arg(ap, bool) was undefined behavior, but didn't realize va_start(ap, bool_param) was also undefined. But sure enough, reading C99 7.15.1.4: 4 The parameter parmN is the identifier of the rightmost parameter in the variable parameter list in the function definition (the one just before the , ...). If the parameter parmN is declared with the register storage class, with a function or array type, or with a type that is not compatible with the type that results after application of the default argument promotions, the behavior is undefined. > Feel free to explain the long version to me, because I don't > think I have fully understood the issue, but it's something like > "Using bools for variadic arguments results in their promotion to > an int, but you have to use a type that is promoted to itself > (like int)." So it must be that C99 is trying to cater to platforms that have special ABI for passing multiple bool on the stack, by stating that the moment argument promotion is in effect, va_list adjacent to a bool may cause problems with that special packing in the ABI. Weird. > > > git-backport-diff against v6: > > Key: > [----] : patches are identical > [####] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, > respectively > > 001/6:[----] [--] 'qapi/qnull: Add own header' > 002/6:[----] [--] 'qapi/qlist: Add qlist_append_null() macro' > 003/6:[----] [--] 'qapi: Add qobject_is_equal()' > 004/6:[----] [--] 'block: qobject_is_equal() in bdrv_reopen_prepare()' > 005/6:[----] [--] 'iotests: Add test for non-string option reopening' > 006/6:[0011] [FC] 'tests: Add check-qobject for equality tests' I still think this is 2.11 material. Once you fix the typo I point out separately on 6/6, the changes since v6 look reasonable, so series: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature