Re: [PATCH resend] s390: oprofile: fix error checks in oprofile_hwsampler_init()

[Robert R. <rob...@am...>]

On 02.05.11 09:48:05, Nicolas Kaiser wrote:
> Checking 'oprofile_min_interval < 0' and
> 'oprofile_max_interval < 0' doesn't work because
> 'oprofile_min_interval' and 'oprofile_max_interval' are unsigned.

max/min_interval are through all the code always unsigned. I don't
know how min/max_sampl_rate in struct hws_qsi_info_block is spec'ed,
but there it is unsigned too.

So the best would be to return qsi.min/max_sampl_rate in
hwsampler_query_min/max_interval() directly with no error codes as
unsigned longs and to change the code in oprofile_hwsampler_init() to
check for null. Both functions hwsampler_query_min/max_interval()
could be moved to hwsampler.h as static inline functions. This makes
the code also easier.

This patch does not handle the null value case and the data truncation
by casting from unsigned to singed is not fixed.

-Robert

> 
> Signed-off-by: Nicolas Kaiser <ni...@ni...>
> ---
> Untested.
> 
>  arch/s390/oprofile/init.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/oprofile/init.c b/arch/s390/oprofile/init.c
> index c63d7e5..09c3088 100644
> --- a/arch/s390/oprofile/init.c
> +++ b/arch/s390/oprofile/init.c
> @@ -138,22 +138,26 @@ static int oprofile_create_hwsampling_files(struct super_block *sb,
>  
>  static int oprofile_hwsampler_init(struct oprofile_operations *ops)
>  {
> +	long retval;
> +
>  	if (hwsampler_setup())
>  		return -ENODEV;
>  
>  	/*
>  	 * create hwsampler files only if hwsampler_setup() succeeds.
>  	 */
> -	oprofile_min_interval = hwsampler_query_min_interval();
> -	if (oprofile_min_interval < 0) {
> +	retval = hwsampler_query_min_interval();
> +	if (retval < 0) {
>  		oprofile_min_interval = 0;
>  		return -ENODEV;
>  	}
> -	oprofile_max_interval = hwsampler_query_max_interval();
> -	if (oprofile_max_interval < 0) {
> +	oprofile_min_interval = retval;
> +	retval = hwsampler_query_max_interval();
> +	if (retval < 0) {
>  		oprofile_max_interval = 0;
>  		return -ENODEV;
>  	}
> +	oprofile_max_interval = retval;
>  
>  	if (oprofile_timer_init(ops))
>  		return -ENODEV;
> -- 
> 1.7.3.4
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center