[PATCH] src: drop some forward declarations in src/storage/storage_backend_sheepdog.c

peili posted 1 patch 3 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210327025447.68576-1-peili@andrew.cmu.edu
src/storage/storage_backend_sheepdog.c | 6 ------
1 file changed, 6 deletions(-)
[PATCH] src: drop some forward declarations in src/storage/storage_backend_sheepdog.c
Posted by peili 3 years ago
---
 src/storage/storage_backend_sheepdog.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index 8c37947308..010e86aa14 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -35,12 +35,6 @@
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
-static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool,
-                                               virStorageVolDefPtr vol);
-
-void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
-                                         virStoragePoolObjPtr pool);
-
 int
 virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
                                        char *output)
-- 
2.20.1

Re: [PATCH] src: drop some forward declarations in src/storage/storage_backend_sheepdog.c
Posted by Peter Krempa 3 years ago
On Sat, Mar 27, 2021 at 10:54:39 +0800, peili wrote:
> ---
>  src/storage/storage_backend_sheepdog.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
> index 8c37947308..010e86aa14 100644
> --- a/src/storage/storage_backend_sheepdog.c
> +++ b/src/storage/storage_backend_sheepdog.c
> @@ -35,12 +35,6 @@
>  
>  #define VIR_FROM_THIS VIR_FROM_STORAGE
>  
> -static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool,
> -                                               virStorageVolDefPtr vol);
> -
> -void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
> -                                         virStoragePoolObjPtr pool);
> -
>  int
>  virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
>                                         char *output)

Fails to compile:

../../../libvirt/src/storage/storage_backend_sheepdog.c:86:1: error: no previous prototype for ‘virStorageBackendSheepdogAddHostArg’ [-Werror=missing-prototypes]
   86 | virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../libvirt/src/storage/storage_backend_sheepdog.c: In function ‘virStorageBackendSheepdogAddVolume’:
../../../libvirt/src/storage/storage_backend_sheepdog.c:121:9: error: implicit declaration of function ‘virStorageBackendSheepdogRefreshVol’; did you mean ‘virStorageBackendSheepdogRegister’? [-Werror=implicit-function-declaration]
  121 |     if (virStorageBackendSheepdogRefreshVol(pool, vol) < 0)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         virStorageBackendSheepdogRegister
../../../libvirt/src/storage/storage_backend_sheepdog.c:121:9: error: nested extern declaration of ‘virStorageBackendSheepdogRefreshVol’ [-Werror=nested-externs]
../../../libvirt/src/storage/storage_backend_sheepdog.c: At top level:
../../../libvirt/src/storage/storage_backend_sheepdog.c:308:1: error: static declaration of ‘virStorageBackendSheepdogRefreshVol’ follows non-static declaration
  308 | virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool,
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../libvirt/src/storage/storage_backend_sheepdog.c:121:9: note: previous implicit declaration of ‘virStorageBackendSheepdogRefreshVol’ was here
  121 |     if (virStorageBackendSheepdogRefreshVol(pool, vol) < 0)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


NACK, the patch doesn't justify this by any means and it's breaking
build.

[PATCH 8/8] src: drop some forward declarations in src/storage/storage_backend_sheepdog.c
Posted by peili 3 years ago
---
 src/storage/storage_backend_sheepdog.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index 8c37947308..010e86aa14 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -35,12 +35,6 @@
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
-static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool,
-                                               virStorageVolDefPtr vol);
-
-void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
-                                         virStoragePoolObjPtr pool);
-
 int
 virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
                                        char *output)
-- 
2.20.1

Re: [PATCH 8/8] src: drop some forward declarations in src/storage/storage_backend_sheepdog.c
Posted by Michal Privoznik 3 years ago
On 3/27/21 3:54 AM, peili wrote:
> ---
>   src/storage/storage_backend_sheepdog.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
> index 8c37947308..010e86aa14 100644
> --- a/src/storage/storage_backend_sheepdog.c
> +++ b/src/storage/storage_backend_sheepdog.c
> @@ -35,12 +35,6 @@
>   
>   #define VIR_FROM_THIS VIR_FROM_STORAGE
>   
> -static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool,
> -                                               virStorageVolDefPtr vol);
> -
> -void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
> -                                         virStoragePoolObjPtr pool);
> -
>   int
>   virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
>                                          char *output)
> 

Unfortunately, dropping forward declarations is not that easy. For 
instance in the following example:

int foo() {
   ...
   bar();
   ...
}


int bar() {
   ...
   foo();
   ...
}


This block has to be prefaced with forward declaration of bar() because 
as compiler parses the input, it sees foo() (among with it declaration), 
and then it sees bar() being called, but since it did not see its 
declaration yet, it can't ensure that arguments passed are correct type, 
correct count, ...;  therefore, bar() has to be declared before before 
foo().

However, consider the following case:

int foo() {
   ...
   // no bar() call here
   ...
}


int bar() {
   ...
   foo();
   ...
}

here, function foo() does NOT call bar() and therefore, there is no need 
for forward declaration of bar().

Michal