From 443454a6212e5596f7caaadf3b666fa4857edb64 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Wed, 13 Feb 2019 23:18:43 +0100 Subject: [PATCH] clarifying namestore api (#5458), fixing code duplication and a memory leak while at it --- src/include/gnunet_namestore_plugin.h | 9 ++- src/namestore/gnunet-service-namestore.c | 45 ++++++----- src/namestore/plugin_namestore_heap.c | 93 +++++++++++------------ src/namestore/plugin_namestore_postgres.c | 1 + src/namestore/plugin_namestore_sqlite.c | 1 + 5 files changed, 79 insertions(+), 70 deletions(-) diff --git a/src/include/gnunet_namestore_plugin.h b/src/include/gnunet_namestore_plugin.h index 46a7da792..3b603b4c0 100644 --- a/src/include/gnunet_namestore_plugin.h +++ b/src/include/gnunet_namestore_plugin.h @@ -11,7 +11,7 @@ WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. - + You should have received a copy of the GNU Affero General Public License along with this program. If not, see . @@ -47,7 +47,8 @@ extern "C" * Function called for each matching record. * * @param cls closure - * @param serial unique serial number of the record + * @param serial unique serial number of the record, MUST NOT BE ZERO, + * and must be monotonically increasing while iterating * @param zone_key private key of the zone * @param label name that is being mapped (at most 255 characters long) * @param rd_count number of entries in @a rd array @@ -115,7 +116,9 @@ struct GNUNET_NAMESTORE_PluginFunctions * * @param cls closure (internal context for the plugin) * @param zone private key of the zone, NULL for all zones - * @param serial serial (to exclude) in the list of matching records + * @param serial serial (to exclude) in the list of matching records; + * 0 means to exclude nothing; results must be returned using + * the minimum possible sequence number first (ordered by serial) * @param limit maximum number of results to return to @a iter * @param iter function to call with the result * @param iter_cls closure for @a iter diff --git a/src/namestore/gnunet-service-namestore.c b/src/namestore/gnunet-service-namestore.c index b1f8fcf4c..45be0fe75 100644 --- a/src/namestore/gnunet-service-namestore.c +++ b/src/namestore/gnunet-service-namestore.c @@ -124,7 +124,7 @@ struct ZoneIteration * message and free the data structure once @e cache_ops is zero. */ int send_end; - + }; @@ -268,7 +268,7 @@ struct CacheOperation * for if applicable, can be NULL. */ struct ZoneIteration *zi; - + /** * Client's request ID. */ @@ -318,12 +318,12 @@ struct StoreActivity /** * Entry in list of cached nick resolutions. - */ + */ struct NickCache { /** * Zone the cache entry is for. - */ + */ struct GNUNET_CRYPTO_EcdsaPrivateKey zone; /** @@ -339,7 +339,7 @@ struct NickCache /** - * We cache nick records to reduce DB load. + * We cache nick records to reduce DB load. */ static struct NickCache nick_cache[NC_SIZE]; @@ -489,7 +489,7 @@ free_store_activity (struct StoreActivity *sa) * record, which (if found) is then copied to @a cls for future use. * * @param cls a `struct GNUNET_GNSRECORD_Data **` for storing the nick (if found) - * @param seq sequence number of the record + * @param seq sequence number of the record, MUST NOT BE ZERO * @param private_key the private key of the zone (unused) * @param label should be #GNUNET_GNS_EMPTY_LABEL_AT * @param rd_count number of records in @a rd @@ -506,7 +506,7 @@ lookup_nick_it (void *cls, struct GNUNET_GNSRECORD_Data **res = cls; (void) private_key; - (void) seq; + GNUNET_assert (0 != seq); if (0 != strcmp (label, GNUNET_GNS_EMPTY_LABEL_AT)) { GNUNET_break (0); @@ -607,7 +607,7 @@ get_nick_record (const struct GNUNET_CRYPTO_EcdsaPrivateKey *zone) return nick; } } - + nick = NULL; res = GSN_database->lookup_records (GSN_database->cls, zone, @@ -872,7 +872,7 @@ zone_iteration_done_client_continue (struct ZoneIteration *zi) em->r_id = htonl (zi->request_id); GNUNET_MQ_send (zi->nc->mq, env); - + GNUNET_CONTAINER_DLL_remove (zi->nc->op_head, zi->nc->op_tail, zi); @@ -1270,9 +1270,16 @@ struct RecordLookupContext /** - * FIXME. + * Function called by the namestore plugin when we are trying to lookup + * a record as part of #handle_record_lookup(). Merges all results into + * the context. * - * @param seq sequence number of the record + * @param cls closure with a `struct RecordLookupContext` + * @param seq unique serial number of the record, MUST NOT BE ZERO + * @param zone_key private key of the zone + * @param label name that is being mapped (at most 255 characters long) + * @param rd_count number of entries in @a rd array + * @param rd array of records with data to store */ static void lookup_it (void *cls, @@ -1285,7 +1292,7 @@ lookup_it (void *cls, struct RecordLookupContext *rlc = cls; (void) private_key; - (void) seq; + GNUNET_assert (0 != seq); if (0 != strcmp (label, rlc->label)) return; @@ -1609,7 +1616,7 @@ handle_record_store (void *cls, conv_name)) || (GNUNET_GNSRECORD_TYPE_NICK != rd[i].record_type) ) rd_clean_off++; - + if ( (0 == strcmp (GNUNET_GNS_EMPTY_LABEL_AT, conv_name)) && (GNUNET_GNSRECORD_TYPE_NICK == rd[i].record_type) ) @@ -1680,7 +1687,7 @@ struct ZoneToNameCtx * Zone to name iterator * * @param cls struct ZoneToNameCtx * - * @param seq sequence number of the record + * @param seq sequence number of the record, MUST NOT BE ZERO * @param zone_key the zone key * @param name name * @param rd_count number of records in @a rd @@ -1704,7 +1711,7 @@ handle_zone_to_name_it (void *cls, char *name_tmp; char *rd_tmp; - (void) seq; + GNUNET_assert (0 != seq); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Found result for zone-to-name lookup: `%s'\n", name); @@ -1822,7 +1829,7 @@ struct ZoneIterationProcResult * Process results for zone iteration from database * * @param cls struct ZoneIterationProcResult - * @param seq sequence number of the record + * @param seq sequence number of the record, MUST NOT BE ZERO * @param zone_key the zone key * @param name name * @param rd_count number of records for this name @@ -1839,6 +1846,7 @@ zone_iterate_proc (void *cls, struct ZoneIterationProcResult *proc = cls; int do_refresh_block; + GNUNET_assert (0 != seq); if ( (NULL == zone_key) && (NULL == name) ) { @@ -1876,7 +1884,7 @@ zone_iterate_proc (void *cls, do_refresh_block = GNUNET_YES; break; } - if (GNUNET_YES == do_refresh_block) + if (GNUNET_YES == do_refresh_block) refresh_block (NULL, proc->zi, 0, @@ -2116,7 +2124,7 @@ monitor_iteration_next (void *cls); * A #GNUNET_NAMESTORE_RecordIterator for monitors. * * @param cls a 'struct ZoneMonitor *' with information about the monitor - * @param seq sequence number of the record + * @param seq sequence number of the record, MUST NOT BE ZERO * @param zone_key zone key of the zone * @param name name * @param rd_count number of records in @a rd @@ -2132,6 +2140,7 @@ monitor_iterate_cb (void *cls, { struct ZoneMonitor *zm = cls; + GNUNET_assert (0 != seq); zm->seq = seq; GNUNET_assert (NULL != name); GNUNET_STATISTICS_update (statistics, diff --git a/src/namestore/plugin_namestore_heap.c b/src/namestore/plugin_namestore_heap.c index 78e99442c..01cf592ea 100644 --- a/src/namestore/plugin_namestore_heap.c +++ b/src/namestore/plugin_namestore_heap.c @@ -79,10 +79,41 @@ struct FlatFileEntry */ char *label; - }; +/** + * Hash contactenation of @a pkey and @a label into @a h + * + * @param pkey a key + * @param label a label + * @param h[out] initialized hash + */ +static void +hash_pkey_and_label (const struct GNUNET_CRYPTO_EcdsaPrivateKey *pkey, + const char *label, + struct GNUNET_HashCode *h) +{ + char *key; + size_t label_len; + size_t key_len; + + label_len = strlen (label); + key_len = label_len + sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey); + key = GNUNET_malloc (key_len); + GNUNET_memcpy (key, + label, + label_len); + GNUNET_memcpy (key + label_len, + pkey, + sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey)); + GNUNET_CRYPTO_hash (key, + key_len, + h); + GNUNET_free (key); +} + + /** * Initialize the database connections and associated * data structures (create tables and indices @@ -262,23 +293,9 @@ database_setup (struct Plugin *plugin) GNUNET_free (private_key); } - { - char *key; - size_t key_len; - - key_len = strlen (label) + sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey); - key = GNUNET_malloc (strlen (label) + sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey)); - GNUNET_memcpy (key, - label, - strlen (label)); - GNUNET_memcpy (key+strlen(label), - &entry->private_key, - sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey)); - GNUNET_CRYPTO_hash (key, - key_len, - &hkey); - GNUNET_free (key); - } + hash_pkey_and_label (&entry->private_key, + label, + &hkey); if (GNUNET_OK != GNUNET_CONTAINER_multihashmap_put (plugin->hm, &hkey, @@ -425,24 +442,14 @@ namestore_heap_store_records (void *cls, { struct Plugin *plugin = cls; uint64_t rvalue; - size_t key_len; - char *key; struct GNUNET_HashCode hkey; struct FlatFileEntry *entry; rvalue = GNUNET_CRYPTO_random_u64 (GNUNET_CRYPTO_QUALITY_WEAK, UINT64_MAX); - key_len = strlen (label) + sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey); - key = GNUNET_malloc (key_len); - GNUNET_memcpy (key, - label, - strlen (label)); - GNUNET_memcpy (key + strlen(label), - zone_key, - sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey)); - GNUNET_CRYPTO_hash (key, - key_len, - &hkey); + hash_pkey_and_label (zone_key, + label, + &hkey); GNUNET_CONTAINER_multihashmap_remove_all (plugin->hm, &hkey); if (0 == rd_count) @@ -501,27 +508,15 @@ namestore_heap_lookup_records (void *cls, struct Plugin *plugin = cls; struct FlatFileEntry *entry; struct GNUNET_HashCode hkey; - char *key; - size_t key_len; if (NULL == zone) { GNUNET_break (0); return GNUNET_SYSERR; } - key_len = strlen (label) + sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey); - key = GNUNET_malloc (key_len); - GNUNET_memcpy (key, - label, - strlen (label)); - GNUNET_memcpy (key+strlen(label), - zone, - sizeof (struct GNUNET_CRYPTO_EcdsaPrivateKey)); - GNUNET_CRYPTO_hash (key, - key_len, - &hkey); - GNUNET_free (key); - + hash_pkey_and_label (zone, + label, + &hkey); entry = GNUNET_CONTAINER_multihashmap_get (plugin->hm, &hkey); @@ -529,7 +524,7 @@ namestore_heap_lookup_records (void *cls, return GNUNET_NO; if (NULL != iter) iter (iter_cls, - 0, + 1, /* zero is illegal */ &entry->private_key, entry->label, entry->record_count, @@ -609,7 +604,7 @@ iterate_zones (void *cls, } ic->iter (ic->iter_cls, ic->pos, - (NULL == ic->zone) + (NULL == ic->zone) ? &entry->private_key : ic->zone, entry->label, @@ -695,7 +690,7 @@ zone_to_name (void *cls, sizeof (struct GNUNET_CRYPTO_EcdsaPublicKey))) { ztn->iter (ztn->iter_cls, - 0, + i + 1, /* zero is illegal! */ &entry->private_key, entry->label, entry->record_count, diff --git a/src/namestore/plugin_namestore_postgres.c b/src/namestore/plugin_namestore_postgres.c index 57a8ae2be..f2e065882 100644 --- a/src/namestore/plugin_namestore_postgres.c +++ b/src/namestore/plugin_namestore_postgres.c @@ -400,6 +400,7 @@ parse_result_call_iterator (void *cls, { struct GNUNET_GNSRECORD_Data rd[GNUNET_NZL(record_count)]; + GNUNET_assert (0 != serial); if (GNUNET_OK != GNUNET_GNSRECORD_records_deserialize (data_size, data, diff --git a/src/namestore/plugin_namestore_sqlite.c b/src/namestore/plugin_namestore_sqlite.c index 96b0d6457..e4bfcde16 100644 --- a/src/namestore/plugin_namestore_sqlite.c +++ b/src/namestore/plugin_namestore_sqlite.c @@ -521,6 +521,7 @@ get_records_and_call_iterator (struct Plugin *plugin, { struct GNUNET_GNSRECORD_Data rd[record_count]; + GNUNET_assert (0 != seq); if (GNUNET_OK != GNUNET_GNSRECORD_records_deserialize (data_size, data, -- 2.25.1