Re: [PATCH v2 2/2] core: Add subscription API for Manufacturer Specific Data

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Alfonso,

On Fri, Oct 10, 2014, Alfonso Acosta wrote:
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -30,6 +30,8 @@
>  #include <glib.h>
>  #include <stdbool.h>
>  
> +#include "eir.h"
> +
>  #define MAX_NAME_LENGTH		248
>  
>  /* Invalid SSP passkey value used to indicate negative replies */
> @@ -138,6 +140,14 @@ struct btd_adapter_pin_cb_iter *btd_adapter_pin_cb_iter_new(
>  void btd_adapter_pin_cb_iter_free(struct btd_adapter_pin_cb_iter *iter);
>  bool btd_adapter_pin_cb_iter_end(struct btd_adapter_pin_cb_iter *iter);
>  
> +typedef void (*btd_msd_cb_t) (struct btd_adapter *adapter,
> +				struct btd_device *dev,
> +				const struct eir_msd *msd);
> +void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
> +					btd_msd_cb_t cb);

In our user space code we try to follow the following principles for
internal header files:

1) The c-file that includes them should also include the prerequisites
2) We don't use multi-include guards

The general idea is that we don't want hidden and implicit dependencies
but prefer having them explicitly spelled out. This practice also helps
detect circular dependencies. For public header files or those of
library-like modules we don't follow this practice (e.g. gdbus/gdbus.h).

As for your patch, I'd suggest to spell out each of the three variables
in your bt_msd_cb_t instead of using the "struct eir_msd" in adapter.h.
That way you don't have a dependency to eir.h from adapter.h.

Johan