Re: [Qemu-devel] [PATCH RFC v2 1/4] block: refactor bdrv_next_node for readability

[Stefan Hajnoczi]

On Mon, Dec 19, 2016 at 04:51:23PM +0800, Dou Liyang wrote:
> make the bdrv_next_node() clearly and add some comments.
> 
> Signed-off-by: Dou Liyang <address@hidden>
> ---
>  block.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 39ddea3..01c9e51 100644
> --- a/block.c
> +++ b/block.c
> @@ -2931,12 +2931,20 @@ bool bdrv_chain_contains(BlockDriverState *top, 
> BlockDriverState *base)
>      return top != NULL;
>  }
>  
> +/*
> + * Return the BlockDriverStates of all the named nodes.

This sentence describes how this function is used in a loop.  The
semantics of one call to this function are different: only a *single*
named node's BlockDriverState is returned.

> + * If @bs is null, return the first one.
> + * Else, return @bs's next sibling, which may be null.
> + *
> + * To iterate over all BlockDriverStates, do
> + * for (bs = bdrv_next_node(NULL); bs; bs = bdrv_next_node(blk)) {
> + *     ...
> + * }
> + */
>  BlockDriverState *bdrv_next_node(BlockDriverState *bs)
>  {
> -    if (!bs) {
> -        return QTAILQ_FIRST(&graph_bdrv_states);
> -    }
> -    return QTAILQ_NEXT(bs, node_list);
> +    return bs ? QTAILQ_NEXT(bs, node_list)
> +            : QTAILQ_FIRST(&graph_bdrv_states);

The conditional operator (?:) is often considered harder to read than an
if statement.  I see no reason to modify the code.

signature.asc
Description: PGP signature