On 4/15/2021 9:48 PM, Stefano Garzarella wrote:
On Thu, Apr 15, 2021 at 05:53:36PM +0800, Zhu Lingshan wrote:
get_config_size() should return the size based on the decected
device type.

Signed-off-by: Zhu Lingshan <lingshan....@intel.com>
---
drivers/vdpa/ifcvf/ifcvf_main.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index cea1313b1a3f..6844c49fe1de 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -347,7 +347,23 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)

static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev)
{
-    return sizeof(struct virtio_net_config);
+    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
+    struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
+    struct pci_dev *pdev = adapter->pdev;
+    size_t size;
+
+    if (vf->dev_type == VIRTIO_ID_NET)
+        size = sizeof(struct virtio_net_config);
+
+    else if (vf->dev_type == VIRTIO_ID_BLOCK)
+        size = sizeof(struct virtio_blk_config);
+
+    else {
+        size = 0;
+        IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type);
+    }

I slightly prefer the switch, but I don't have a strong opinion.

However, if we want to use if/else, we should follow `Documentation/process/coding-style.rst` line 166:    Note that the closing brace is empty on a line of its own, **except** in    the cases where it is followed by a continuation of the same statement,    ie a ``while`` in a do-statement or an ``else`` in an if-statement, like

also `scripts/checkpatch.pl --strict` complains:

   CHECK: braces {} should be used on all arms of this statement
   #209: FILE: drivers/vdpa/ifcvf/ifcvf_main.c:355:
   +    if (vf->dev_type == VIRTIO_ID_NET)
   [...]
   +    else if (vf->dev_type == VIRTIO_ID_BLOCK)
   [...]
   +    else {
   [...]

   CHECK: Unbalanced braces around else statement
   #215: FILE: drivers/vdpa/ifcvf/ifcvf_main.c:361:
   +    else {
Thanks Stefano, the reason is we only have one line code after if, so looks like {} is unnecessary, I agree switch can clear up
code style confusions. I will add this in v3.

Thanks!

Thanks,
Stefano

_______________________________________________
Virtualization mailing list
virtualizat...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to