Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1326451712-21784-1-git-send-email-lkslawek@gmail.com> Date: Fri, 13 Jan 2012 13:37:50 +0100 Message-ID: Subject: Re: [PATCH obexd 1/4] MAP: Implementation of MAP AP core functions From: Slawomir Bochenski To: Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: On Fri, Jan 13, 2012 at 1:02 PM, Luiz Augusto von Dentz wrote: > Hi Slawomir, > > On Fri, Jan 13, 2012 at 12:48 PM, Slawomir Bochenski 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