On 09/10/13 13:49, Lee Jones wrote:
> Gets rid of those unnecessary gotos.

Unfortunately it introduced a bug whilst it is at it.  Sometimes
those gotos are necessary and the 'right' way to do things.
> 
> Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> ---
>  drivers/iio/common/st_sensors/st_sensors_core.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c 
> b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 8c4c54c..148f0e5 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -331,26 +331,23 @@ static int st_sensors_read_axis_data(struct iio_dev 
> *indio_dev,
>       unsigned int byte_for_channel = ch->scan_type.storagebits >> 3;
>  
>       outdata = kmalloc(byte_for_channel, GFP_KERNEL);
> -     if (!outdata) {
> -             err = -EINVAL;
> -             goto st_sensors_read_axis_data_error;
> -     }
> +     if (!outdata)
> +             return -ENOMEM;
I agree this change makes complete sense.
>  
>       err = sdata->tf->read_multiple_byte(&sdata->tb, sdata->dev,
>                               ch->address, byte_for_channel,
>                               outdata, sdata->multiread_bit);
> -     if (err < 0)
> -             goto st_sensors_free_memory;
> +     if (err < 0) {
> +             kfree(outdata);
> +             return err;
> +     }
I don't like this change as....
>  
>       if (byte_for_channel == 2)
>               *data = (s16)get_unaligned_le16(outdata);
>       else if (byte_for_channel == 3)
>               *data = (s32)st_sensors_get_unaligned_le24(outdata);
>  
> -st_sensors_free_memory:
> -     kfree(outdata);
you now don't free out data in the case of no error.  This is precisely the case
where a goto is the correct way to handle things as the free need to occur 
whether
or not the error occurs.

> -st_sensors_read_axis_data_error:
> -     return err;
> +     return 0;
>  }
>  
>  int st_sensors_read_info_raw(struct iio_dev *indio_dev,
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to