[PATCH BlueZ] mesh: Clean up includes

[PATCH BlueZ] mesh: Clean up includes

[Inga Stotland]

This adds #include for json-c/json.h in mesh-db.h and removes this
include from the other files that don't need to reference json-c.
---
 mesh/cfgmod-server.c | 2 --
 mesh/mesh-db.c       | 1 -
 mesh/mesh-db.h       | 2 ++
 mesh/model.c         | 2 --
 mesh/node.c          | 1 -
 mesh/storage.c       | 1 -
 6 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 060d7f4e4..2f61d841c 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -24,8 +24,6 @@
 #include <sys/time.h>
 #include <ell/ell.h>
 
-#include "json-c/json.h"
-
 #include "mesh/mesh-defs.h"
 #include "mesh/node.h"
 #include "mesh/net.h"
diff --git a/mesh/mesh-db.c b/mesh/mesh-db.c
index e0a000261..f3807070f 100644
--- a/mesh/mesh-db.c
+++ b/mesh/mesh-db.c
@@ -27,7 +27,6 @@
 #include <string.h>
 
 #include <ell/ell.h>
-#include <json-c/json.h>
 
 #include "mesh/mesh-defs.h"
 #include "mesh/util.h"
diff --git a/mesh/mesh-db.h b/mesh/mesh-db.h
index da5efa12a..8fb0eb291 100644
--- a/mesh/mesh-db.h
+++ b/mesh/mesh-db.h
@@ -17,6 +17,8 @@
  *
  */
 
+#include <json-c/json.h>
+
 struct mesh_db_sub {
 	bool virt;
 	union {
diff --git a/mesh/model.c b/mesh/model.c
index f29ad9af2..4b5605af6 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -23,10 +23,8 @@
 
 #include <sys/time.h>
 #include <ell/ell.h>
-#include <json-c/json.h>
 
 #include "mesh/mesh-defs.h"
-
 #include "mesh/mesh.h"
 #include "mesh/crypto.h"
 #include "mesh/node.h"
diff --git a/mesh/node.c b/mesh/node.c
index e99858623..c8d0fac2e 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -26,7 +26,6 @@
 #include <sys/time.h>
 
 #include <ell/ell.h>
-#include <json-c/json.h>
 
 #include "mesh/mesh-defs.h"
 #include "mesh/mesh.h"
diff --git a/mesh/storage.c b/mesh/storage.c
index 1a9945aa8..6cb280c39 100644
--- a/mesh/storage.c
+++ b/mesh/storage.c
@@ -30,7 +30,6 @@
 #include <libgen.h>
 #include <ftw.h>
 
-#include <json-c/json.h>
 #include <ell/ell.h>
 
 #include "mesh/mesh-defs.h"
-- 
2.21.0

Re: [PATCH BlueZ] mesh: Clean up includes

[Michał Lowas-Rzechonek]

Hi Inga,

On 06/17, Inga Stotland wrote:
> This adds #include for json-c/json.h in mesh-db.h and removes this
> include from the other files that don't need to reference json-c.

While I agree about removal from cfgmod-server, model and node, I don't
think we should remove the include from storage and move it to mesh-db.

I'd rather see #includes follow https://include-what-you-use.org/
approach, that is:

    (...) for every symbol (type, function variable, or macro) that you
    use in foo.cc, either foo.cc or foo.h should #include a .h file that
    exports the declaration of that symbol.

Moreover, I think headers should only be included as-needed (mostly in
.c files), and headers should contain forward declarations of various
types, to make them opaque.

Such an approach cuts implicit dependencies, making code maintenance
easier: when, for example, someone decides to refactor mesh-db not to
use json_object anymore, they shouldn't need to suddenly deal with
missing declarations in other modules (in this case, storage).

Moreover, each header should be self-sufficient: in order to use API
exposed by a given header, it should be enough to include that header
only. At the moment, this is not the case.

A minor side-effect would be faster builds and less rebuilds when code
is being worked on.

If you agree, I am more than happy to fix this throughout the mesh/
codebase.


regards
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

Re: [PATCH BlueZ] mesh: Clean up includes

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

Hi Michal,

On Tue, 2019-06-18 at 09:35 +0200, Michał Lowas-Rzechonek wrote:
> Hi Inga,
> 
> On 06/17, Inga Stotland wrote:
> > This adds #include for json-c/json.h in mesh-db.h and removes this
> > include from the other files that don't need to reference json-c.
> 
> While I agree about removal from cfgmod-server, model and node, I
> don't
> think we should remove the include from storage and move it to mesh-
> db.
> 
> I'd rather see #includes follow https://include-what-you-use.org/
> approach, that is:
> 
>     (...) for every symbol (type, function variable, or macro) that
> you
>     use in foo.cc, either foo.cc or foo.h should #include a .h file
> that
>     exports the declaration of that symbol.
> 
> Moreover, I think headers should only be included as-needed (mostly
> in
> .c files), and headers should contain forward declarations of various
> types, to make them opaque.
> 
> Such an approach cuts implicit dependencies, making code maintenance
> easier: when, for example, someone decides to refactor mesh-db not to
> use json_object anymore, they shouldn't need to suddenly deal with
> missing declarations in other modules (in this case, storage).
> 
> Moreover, each header should be self-sufficient: in order to use API
> exposed by a given header, it should be enough to include that header
> only. At the moment, this is not the case.
> 
> A minor side-effect would be faster builds and less rebuilds when
> code
> is being worked on.
> 
> If you agree, I am more than happy to fix this throughout the mesh/
> codebase.
> 

I don't disagree, this was sort of an "intermediate" patch. I'd prefer
to remove the reference to json from storage.c and the rest of the code
 and localize it in only one file that would deal with json-related
processing.

So I might just as well go ahead and make mesh-db.h a generic API to
allow for different storage formats (even though for now we have only
json defined format).

Best regards,

Inga

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

Re: [PATCH BlueZ] mesh: Clean up includes

[michal.lowas-rzechonek]

Hi Inga,

On 06/18, Stotland, Inga wrote:
> I don't disagree, this was sort of an "intermediate" patch. I'd prefer
> to remove the reference to json from storage.c and the rest of the code
> and localize it in only one file that would deal with json-related
> processing.
> 
> So I might just as well go ahead and make mesh-db.h a generic API to
> allow for different storage formats (even though for now we have only
> json defined format).

Sounds good! If that's the goal, please go ahead with the patch and any
further ones. I'm very much interested in using non-JSON storage, as
more granular format might be better suited for embedded devices (flash
wear, power failures and whatnot).

FYI, you've probably noticed I'm attempting a similar thing with mesh-io
layer.

best regards
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND