On 11/21/19 12:17 PM, Cornelia Huck wrote:
> On Wed, 20 Nov 2019 06:43:21 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Let's improve readability by:
>> * Using constants for the subcodes
>> * Moving parameter checking into a function
>> * Removing subcode > 6 check as the default case catches that
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  target/s390x/diag.c | 54 +++++++++++++++++++++++++++------------------
>>  1 file changed, 32 insertions(+), 22 deletions(-)
>>
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index 53c2f81f2a..067c667ba7 100644
>> --- a/target/s390x/diag.c
>> +++ b/target/s390x/diag.c
>> @@ -53,6 +53,29 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>>  #define DIAG_308_RC_NO_CONF         0x0102
>>  #define DIAG_308_RC_INVALID         0x0402
>>  
>> +#define DIAG308_RES_MOD_CLR		0
>> +#define DIAG308_RES_LOAD_NORM		1
> 
> Maybe s/RES/RESET/ ? Not that much longer, but clearer IMHO.

Sure

> 
>> +#define DIAG308_LOAD_CLEAR		3
>> +#define DIAG308_LOAD_NORMAL_DUMP	4
>> +#define DIAG308_SET			5
>> +#define DIAG308_STORE			6
> 
> (...)
> 
> Really more readable afterwards.
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Thanks!