Re: [PATCH v8 1/2] s390x: ipl: Consolidate iplb validity check into one function

[Christian Borntraeger]

On 10.03.20 09:32, Janosch Frank wrote:
> It's nicer to just call one function than calling a function for each
> possible iplb type.
> 
> Signed-off-by: Janosch Frank <address@hidden>

> ---
>  hw/s390x/ipl.h      | 20 +++++++++++---------
>  target/s390x/diag.c |  2 +-
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index d4813105db..211ce2dbeb 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -173,16 +173,18 @@ static inline bool iplb_valid_len(IplParameterBlock 
> *iplb)
>      return be32_to_cpu(iplb->len) <= sizeof(IplParameterBlock);
>  }
>  
> -static inline bool iplb_valid_ccw(IplParameterBlock *iplb)
> +static inline bool iplb_valid(IplParameterBlock *iplb)
>  {
> -    return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN &&
> -           iplb->pbt == S390_IPL_TYPE_CCW;
> -}
> -
> -static inline bool iplb_valid_fcp(IplParameterBlock *iplb)
> -{
> -    return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN &&
> -           iplb->pbt == S390_IPL_TYPE_FCP;
> +    switch (iplb->pbt) {
> +    case S390_IPL_TYPE_FCP:
> +        return (be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN &&
> +                iplb->pbt == S390_IPL_TYPE_FCP);

Isnt the iplb->pbt check redundant due to the switch statement?

> +    case S390_IPL_TYPE_CCW:
> +        return (be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN &&
> +                iplb->pbt == S390_IPL_TYPE_CCW);
> +    default:

same here.