Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 10 Jul 2019 09:38:27 +0200
From:   =?utf-8?Q?Micha=C5=82?= Lowas-Rzechonek 
        <michal.lowas-rzechonek@silvair.com>
To:     Inga Stotland <inga.stotland@intel.com>
Cc:     linux-bluetooth@vger.kernel.org, brian.gix@intel.com,
        jakub.witowski@silvair.com
Subject: Re: [PATCH BlueZ 4/9] mesh: Generalize mesh-config APIs
Message-ID: <20190710073827.5ktzlyynztmebiv6@mlowasrzechonek2133>
Mail-Followup-To: Inga Stotland <inga.stotland@intel.com>,
        linux-bluetooth@vger.kernel.org, brian.gix@intel.com,
        jakub.witowski@silvair.com
References: <20190710050959.7321-1-inga.stotland@intel.com>
 <20190710050959.7321-5-inga.stotland@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190710050959.7321-5-inga.stotland@intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Inga,

On 07/09, Inga Stotland wrote:
> This changes argument for each mesh-config API to use void* as
> a pointer to a node configuration object. This makes usage of JSON
> opaque for the rest of the code and allows to plug in a non-JSON
> configuration storage implementation.
> ---
>  mesh/mesh-config-json.c | 228 +++++++++++++++++++++++-----------------
>  mesh/mesh-config.h      | 107 +++++++++----------
>  2 files changed, 180 insertions(+), 155 deletions(-)
> 
> diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
> index 8fcb8afe3..5ca086ad0 100644
> --- a/mesh/mesh-config-json.c
> +++ b/mesh/mesh-config-json.c
> @@ -252,10 +252,14 @@ static json_object *jarray_key_del(json_object *jarray, int16_t idx)
>  	return jarray_new;
>  }
>  
> -bool mesh_config_read_iv_index(json_object *jobj, uint32_t *idx, bool *update)
> +bool mesh_config_read_iv_index(void *cfg, uint32_t *idx, bool *update)

I'm not a fan of using void* for polymorphism. While there are places
this makes sense (e.g. user_data pointers in callbacks), I think we
should try to avoid this in our APIs.

Since the proposed patch assumes that we would switch backends during
the build, and you can't have two different config formats compiled in
at the same time, how about this:

mesh-config.h:
    union mesh_config;

    union mesh_config *mesh_config_create_config(void);
    void mesh_config_release_config(union mesh_config *config);

mesh-config-json.c:
    union mesh_config {
        json_object *json
    };

    union mesh_config *mesh_config_create_config(void)
    {
        return (union mesh_config*)json_object_new_object();
    }

    void mesh_config_release_config(union mesh_config *cfg)
    {
        json_object *jnode = (json_object *)cfg;

        if (!cfg)
            return;

        json_object_put(jnode);
    }

etc.

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