Re: [PATCH 2/7] ios: Pass offset to low-level macros.

[Eric Blake]

On 2/29/20 5:12 AM, Eric Blake wrote:
> This patch should have no semantic change, although it might have a
> slight pessimization in performance which will be cleaned up in later
> patches.
>
> * src/ios.c (IOS_GET_C_ERR_CHCK, IOS_PUT_C_ERR_CHCK)
> (IOS_READ_INTO_CHARRAY_1BYTE, IOS_READ_INTO_CHARRAY_2BYTES)
> (IOS_READ_INTO_CHARRAY_3BYTES, IOS_READ_INTO_CHARRAY_4BYTES)
> (IOS_READ_INTO_CHARRAY_5BYTES, IOS_READ_INTO_CHARRAY_6BYTES)
> (IOS_READ_INTO_CHARRAY_7BYTES, IOS_READ_INTO_CHARRAY_8BYTES)
> (IOS_READ_INTO_CHARRAY_9BYTES): Add parameter (unused until next
> patch).
> (ios_write_int_fast): Add parameter.
> (ios_read_int_common, ios_read_int, ios_read_uint)
> (ios_write_int_common, ios_write_int, ios_write_uint): Adjust
> callers.
> ---
>   ChangeLog |  15 +++
>   src/ios.c | 395 +++++++++++++++++++++++++++---------------------------
>   2 files changed, 213 insertions(+), 197 deletions(-)

> +++ b/src/ios.c
> @@ -32,7 +32,7 @@
>
>   #define STREQ(a, b) (strcmp (a, b) == 0)
>
> -#define IOS_GET_C_ERR_CHCK(c, io)               \
> +#define IOS_GET_C_ERR_CHCK(c, io, off)         \
>     {                                             \
>     int ret = io->dev_if->get_c ((io)->dev);   \

Just noticing: We're inconsistent on whether the 'io' argument to this 
macro is allowed to be an arbitrary expression.  Fortunately, none of 
the callers pass a complex expression, but the correct spelling for 
arbitrary syntax would be:

int ret = (io)->dev_if->get_c ((io)->dev); \

yet that still would result in multiple side effects; so even more 
reusable would be:

ios io_ = (io); \
int ret = io_->dev_if->get_c (io_->dev); \

Or, we can declare that this macro is not intended to be reusable 
outside this file, and could spell it:

int ret = io->dev_if->get_c (io->dev); \

or go one step further and declare that it is only to be run inside a 
function where 'io' is already in scope, and not pass in an io argument 
to the macro.

I don't have any strong preferences, but am happy to tweak this and 
subsequent patches to match any style that someone expresses a 
preference for.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org