mbox series

[0/2] Refine some vdi code

Message ID 20180730024701.7613-1-yuchenlin@synology.com (mailing list archive)
Headers show
Series Refine some vdi code | expand

Message

Denis V. Lunev" via July 30, 2018, 2:46 a.m. UTC
From: yuchenlin <yuchenlin@synology.com>

This series refine some code in vdi.c, includes:

* Remvoe CONFIG_VDI_WRITE because there is no reason to leave an always on
  and cannot configure option in the code-side.
* decouple if else if chain to get more readability.

Thanks,
yuchenlin

yuchenlin (2):
  vdi: remove CONFIG_VDI_WRITE
  vdi: refine code for vdi_open

 block/vdi.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

Comments

Stefan Weil July 30, 2018, 7:12 a.m. UTC | #1
Am 30.07.2018 um 04:46 schrieb yuchenlin@synology.com:
> From: yuchenlin <yuchenlin@synology.com>
> 
> This series refine some code in vdi.c, includes:
> 
> * Remvoe CONFIG_VDI_WRITE because there is no reason to leave an always on
>   and cannot configure option in the code-side.
> * decouple if else if chain to get more readability.
> 
> Thanks,
> yuchenlin
> 
> yuchenlin (2):
>   vdi: remove CONFIG_VDI_WRITE
>   vdi: refine code for vdi_open
> 
>  block/vdi.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)


Technically these changes are fine, but personally I prefer my old code.
If else is rendundant here, but redundancy helps humans to understand
the code. CONFIG_VDI_WRITE still has a similar function as it documents
which code parts are relevant for write support.

Stefan
Denis V. Lunev" via July 30, 2018, 7:49 a.m. UTC | #2
Hi, Stefan

I agree that redundancy of If else may helps people to understand the code.

However, CONFIG_VDI_WRITE only contributes:

#if defined(CONFIG_VDI_WRITE)
.bdrv_co_pwritev = vdi_co_pwritev,
#endif

I think we don't need CONFIG_VDI_WRITE to document the code.
As its name implies, vdi_co_pwritev shows the code parts for vdi write support.


I appreciated your time and effort for reviews.

Regards,
yuchenlin


Stefan Weil <sw@weilnetz.de> 於 2018-07-30 15:13 寫道:
> Am 30.07.2018 um 04:46 schrieb yuchenlin@synology.com: > From: yuchenlin <yuchenlin@synology.com> > > This series refine some code in vdi.c, includes: > > * Remvoe CONFIG_VDI_WRITE because there is no reason to leave an always on > and cannot configure option in the code-side. > * decouple if else if chain to get more readability. > > Thanks, > yuchenlin > > yuchenlin (2): > vdi: remove CONFIG_VDI_WRITE > vdi: refine code for vdi_open > > block/vdi.c | 32 ++++++++++++++++++-------------- > 1 file changed, 18 insertions(+), 14 deletions(-) Technically these changes are fine, but personally I prefer my old code. If else is rendundant here, but redundancy helps humans to understand the code. CONFIG_VDI_WRITE still has a similar function as it documents which code parts are relevant for write support. Stefan