> From: Gerd Bayer <gbayer@xxxxxxxxxxxxx>
> Sent: Friday, April 26, 2024 12:56 AM
>
> @@ -131,6 +131,20 @@ VFIO_IORDWR(32)
> VFIO_IORDWR(64)
> #endif
>
> +static int fill_size(size_t fillable, loff_t off)
> +{
> + unsigned int fill_size;
> +#if defined(ioread64) && defined(iowrite64)
> + for (fill_size = 8; fill_size >= 0; fill_size /= 2) {
> +#else
> + for (fill_size = 4; fill_size >= 0; fill_size /= 2) {
> +#endif /* defined(ioread64) && defined(iowrite64) */
> + if (fillable >= fill_size && !(off % fill_size))
> + return fill_size;
> + }
> + return -1;
this is unreachable with "fill_size >= 0" in the loop.
shouldn't it be:
#if defined(ioread64) && defined(iowrite64)
for (fill_size = 8; fill_size > 0; fill_size /= 2) {
#else
for (fill_size = 4; fill_size > 0; fill_size /= 2) {
#endif /* defined(ioread64) && defined(iowrite64) */
if (fillable >= fill_size && !(off % fill_size))
break;
}
return fill_size;
>
> + switch (fill_size(fillable, off)) {
> #if defined(ioread64) && defined(iowrite64)
> - if (fillable >= 8 && !(off % 8)) {
> + case 8:
> ret = vfio_pci_core_iordwr64(vdev, iswrite, test_mem,
> io, buf, off, &filled);
> if (ret)
> return ret;
> + break;
the check on 'ret' can be moved out of the switch to be generic.
