Re: [PATCH 5/7] hw/ide/atapi: Replace magic '512' value by BDRV_SECTOR_SIZE

[Richard Henderson]

On 8/14/20 1:28 AM, Philippe Mathieu-Daudé wrote:
> Use self-explicit definitions instead of magic '512' value.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/ide/atapi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 17a9d635d8..14a2b0bb2f 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -824,9 +824,9 @@ static void cmd_get_configuration(IDEState *s, uint8_t 
> *buf)
>       *
>       *      Only a problem if the feature/profiles grow.
>       */
> -    if (max_len > 512) {
> +    if (max_len > BDRV_SECTOR_SIZE) {
>          /* XXX: assume 1 sector */
> -        max_len = 512;
> +        max_len = BDRV_SECTOR_SIZE;
>      }
>  
>      memset(buf, 0, max_len);
> @@ -1186,8 +1186,8 @@ static void cmd_read_dvd_structure(IDEState *s, 
> uint8_t* buf)
>          }
>      }
>  
> -    memset(buf, 0, max_len > IDE_DMA_BUF_SECTORS * 512 + 4 ?
> -           IDE_DMA_BUF_SECTORS * 512 + 4 : max_len);
> +    memset(buf, 0, max_len > IDE_DMA_BUF_SECTORS * BDRV_SECTOR_SIZE + 4 ?
> +           IDE_DMA_BUF_SECTORS * BDRV_SECTOR_SIZE + 4 : max_len);

If you're queuing other cleanups, both of these places could usefully use MIN().


r~