On Fri, Jan 13, 2012 at 1:02 PM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi Slawomir,
>
> On Fri, Jan 13, 2012 at 12:48 PM, Slawomir Bochenski <lkslawek@xxxxxxxxx> wrote:
>> map_ap_t *map_ap_new(void)
>> {
>> - return NULL;
>> + return g_hash_table_new_full(NULL, NULL, NULL, ap_entry_free);
>
> I supposed you would like to use g_direct_hash and g_direct_equal
> here, even if NULL mean the same it easier to understand and maintain
> that way. Also it is not clear why the return of map_ap_new is
> map_ap_t not a GHashTable?
_I_ would like to use NULL - as you said yourself this is easier to
understand and maintain. And this is explicitly documented default
behavior. map_ap_t was agreed earlier for the implementation to be
interchangeable - map_ap_t is opaque from the map_ap_* API user point
of view.
>
>> }
>>
>> void map_ap_free(map_ap_t *ap)
>> {
>> + if (!ap)
>> + return;
>
> Are you sure you need this check, iirc most functions of glib does
> NULL check already and sometime it even print warnings, so if this
> check is to prevent warning I would suggest no to have because it
> will hide real bugs behind it.
Destroy function here would print critical error message on NULL
GHashTable. I like map_ap_free(NULL) call to be valid just like
g_free(NULL).
>
> --
> Luiz Augusto von Dentz
--
Slawomir Bochenski
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
