From b20a41aaa09caa197e789584e5ad2b0f29b143f1 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Thu, 28 Jun 2012 20:58:15 +0000 Subject: [PATCH] -more namestore service cleanup -- wip --- src/namestore/gnunet-service-namestore.c | 142 ++++++++++++---------- src/namestore/namestore.h | 10 ++ src/namestore/namestore_api.c | 8 ++ src/namestore/plugin_namestore_postgres.c | 3 + src/namestore/plugin_namestore_sqlite.c | 1 - 5 files changed, 99 insertions(+), 65 deletions(-) diff --git a/src/namestore/gnunet-service-namestore.c b/src/namestore/gnunet-service-namestore.c index 2faaaa3ab..83cad685b 100644 --- a/src/namestore/gnunet-service-namestore.c +++ b/src/namestore/gnunet-service-namestore.c @@ -1150,9 +1150,6 @@ handle_record_create (void *cls, } -///////////////////////////////////////////////////////////// - - /** * Context for record remove operations passed from 'handle_record_remove' to * 'handle_record_remove_it' as closure @@ -1200,7 +1197,7 @@ handle_record_remove_it (void *cls, struct RemoveRecordContext *rrc = cls; unsigned int c; int found; - unsigned int rd_count_new; + struct GNUNET_CRYPTO_ShortHashCode pubkey_hash; GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Name `%s 'currently has %u records\n", @@ -1213,9 +1210,11 @@ handle_record_remove_it (void *cls, } /* Find record to remove */ - found = GNUNET_SYSERR; + found = -1; for (c = 0; c < rd_count; c++) { + /* FIXME: shouldn't we test for all fields to match? Otherwise + we might remove the wrong record, just because the type matches! */ /* if (rd[c].flags != rrc->rd->flags) continue;*/ @@ -1232,17 +1231,14 @@ handle_record_remove_it (void *cls, found = c; break; } - if (GNUNET_SYSERR == found) + if (-1 == found) { /* Could not find record to remove */ rrc->op_res = RECORD_REMOVE_RESULT_RECORD_NOT_FOUND; return; } - if (1 == rd_count) { - struct GNUNET_CRYPTO_ShortHashCode pubkey_hash; - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "No records left for name `%s', removing name\n", name); @@ -1262,31 +1258,28 @@ handle_record_remove_it (void *cls, return; } - rd_count_new = rd_count - 1; - struct GNUNET_NAMESTORE_RecordData rd_new[rd_count_new]; - unsigned int c2 = 0; - for (c = 0; c < rd_count; c++) { - if (c != found) + struct GNUNET_NAMESTORE_RecordData rd_new[rd_count - 1]; + unsigned int c2 = 0; + + for (c = 0; c < rd_count; c++) { - GNUNET_assert (c2 < rd_count_new); - rd_new[c2] = rd[c]; - c2++; + if (c == found) + continue; + rd_new[c2++] = rd[c]; + } + if (GNUNET_OK != + GSN_database->put_records(GSN_database->cls, + zone_key, + expire, + name, + rd_count - 1, rd_new, + &dummy_signature)) + { + /* Could not put records into database */ + rrc->op_res = RECORD_REMOVE_RESULT_FAILED_TO_PUT_UPDATE; + return; } - } - - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Name `%s' now has %u records\n", name, rd_count_new); - if (GNUNET_OK != - GSN_database->put_records(GSN_database->cls, - zone_key, - expire, - name, - rd_count_new, rd_new, - &dummy_signature)) - { - /* Could not put records into database */ - rrc->op_res = RECORD_REMOVE_RESULT_FAILED_TO_PUT_UPDATE; - return; } rrc->op_res = RECORD_REMOVE_RESULT_SUCCESS; } @@ -1326,7 +1319,9 @@ handle_record_remove (void *cls, struct RemoveRecordContext rrc; int res; - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Received `%s' message\n", "NAMESTORE_RECORD_REMOVE"); + GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, + "Received `%s' message\n", + "NAMESTORE_RECORD_REMOVE"); if (ntohs (message->size) < sizeof (struct RecordRemoveMessage)) { GNUNET_break (0); @@ -1385,18 +1380,21 @@ handle_record_remove (void *cls, "Received new private key for zone `%s'\n", GNUNET_short_h2s (&pubkey_hash)); cc = GNUNET_malloc (sizeof (struct GNUNET_NAMESTORE_CryptoContainer)); - cc->privkey = GNUNET_CRYPTO_rsa_decode_key (pkey_tmp, key_len); + cc->privkey = pkey; + pkey = NULL; cc->zone = pubkey_hash; GNUNET_assert (GNUNET_YES == GNUNET_CONTAINER_multihashmap_put (zonekeys, &long_hash, cc, GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY)); } + if (NULL != pkey) + GNUNET_CRYPTO_rsa_key_free (pkey); + if (GNUNET_OK != GNUNET_NAMESTORE_records_deserialize (rd_ser_len, rd_ser, rd_count, &rd)) { GNUNET_break (0); - GNUNET_CRYPTO_rsa_key_free (pkey); GNUNET_SERVER_receive_done (client, GNUNET_SYSERR); return; } @@ -1428,12 +1426,22 @@ handle_record_remove (void *cls, name_tmp, 0, handle_record_remove_it, &rrc); - - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, - "Removing record for name `%s': %s\n", - name_tmp, - (0 == rrc.op_res) ? "OK" : "FAIL"); - res = rrc.op_res; + switch (res) + { + case GNUNET_OK: + res = rrc.op_res; + break; + case GNUNET_NO: + res = RECORD_REMOVE_RESULT_NO_RECORDS; + break; + case GNUNET_SYSERR: + res = RECORD_REMOVE_RESULT_FAILED_ACCESS_DATABASE; + break; + default: + GNUNET_break (0); + res = RECORD_REMOVE_RESULT_FAILED_INTERNAL_ERROR; + break; + } } GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Sending `%s' message\n", @@ -1445,7 +1453,6 @@ handle_record_remove (void *cls, GNUNET_SERVER_notification_context_unicast (snc, nc->client, &rrr_msg.gns_header.header, GNUNET_NO); - GNUNET_CRYPTO_rsa_key_free (pkey); GNUNET_SERVER_receive_done (client, GNUNET_OK); } @@ -1462,12 +1469,14 @@ struct ZoneToNameCtx struct GNUNET_NAMESTORE_Client *nc; /** - * Request id + * Request id (to be used in the response to the client). */ uint32_t rid; /** - * Set to GNUNET_OK on success, GNUNET_SYSERR on error + * Set to GNUNET_OK on success, GNUNET_SYSERR on error. Note that + * not finding a name for the zone still counts as a 'success' here, + * as this field is about the success of executing the IPC protocol. */ int success; }; @@ -1503,29 +1512,27 @@ handle_zone_to_name_it (void *cls, char *rd_tmp; char *sig_tmp; - if ((zone_key != NULL) && (name != NULL)) + if ((NULL != zone_key) && (NULL != name)) { /* found result */ - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Found results: name is `%s', has %u records\n", name, rd_count); + GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, + "Found result: name `%s' has %u records\n", + name, rd_count); res = GNUNET_YES; name_len = strlen (name) + 1; } else { /* no result found */ - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Found no results\n"); + GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, + "Found no results\n"); res = GNUNET_NO; name_len = 0; } - - if (rd_count > 0) - rd_ser_len = GNUNET_NAMESTORE_records_get_size (rd_count, rd); - else - rd_ser_len = 0; - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Sending `%s' message\n", "ZONE_TO_NAME_RESPONSE"); + rd_ser_len = GNUNET_NAMESTORE_records_get_size (rd_count, rd); msg_size = sizeof (struct ZoneToNameResponseMessage) + name_len + rd_ser_len; if (NULL != signature) msg_size += sizeof (struct GNUNET_CRYPTO_RsaSignature); @@ -1581,30 +1588,35 @@ handle_zone_to_name (void *cls, GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Received `%s' message\n", "ZONE_TO_NAME"); - if (ntohs (message->size) != sizeof (struct ZoneToNameMessage)) - { - GNUNET_break (0); - GNUNET_SERVER_receive_done (client, GNUNET_SYSERR); - return; - } + ztn_msg = (const struct ZoneToNameMessage *) message; if (NULL == (nc = client_lookup(client))) { GNUNET_break (0); GNUNET_SERVER_receive_done (client, GNUNET_SYSERR); return; } - ztn_msg = (const struct ZoneToNameMessage *) message; ztn_ctx.rid = ntohl (ztn_msg->gns_header.r_id); ztn_ctx.nc = nc; ztn_ctx.success = GNUNET_SYSERR; - GSN_database->zone_to_name (GSN_database->cls, - &ztn_msg->zone, - &ztn_msg->value_zone, - &handle_zone_to_name_it, &ztn_ctx); + if (GNUNET_SYSERR == + GSN_database->zone_to_name (GSN_database->cls, + &ztn_msg->zone, + &ztn_msg->value_zone, + &handle_zone_to_name_it, &ztn_ctx)) + { + /* internal error, hang up instead of signalling something + that might be wrong */ + GNUNET_break (0); + GNUNET_SERVER_receive_done (client, GNUNET_SYSERR); + return; + } GNUNET_SERVER_receive_done (client, ztn_ctx.success); } +///////////////////////////////////////////////////////////// + + /** * Copy record, data has to be free'd separetely * @@ -1814,6 +1826,8 @@ zone_iteraterate_proc (void *cls, return; } } + + ///////////////////////////////////////////////////////////// @@ -2182,7 +2196,7 @@ run (void *cls, struct GNUNET_SERVER_Handle *server, {&handle_record_remove, NULL, GNUNET_MESSAGE_TYPE_NAMESTORE_RECORD_REMOVE, 0}, {&handle_zone_to_name, NULL, - GNUNET_MESSAGE_TYPE_NAMESTORE_ZONE_TO_NAME, 0}, + GNUNET_MESSAGE_TYPE_NAMESTORE_ZONE_TO_NAME, sizeof (struct ZoneToNameMessage) }, {&handle_iteration_start, NULL, GNUNET_MESSAGE_TYPE_NAMESTORE_ZONE_ITERATION_START, sizeof (struct ZoneIterationStartMessage) }, {&handle_iteration_next, NULL, diff --git a/src/namestore/namestore.h b/src/namestore/namestore.h index 990deab1e..ca4dae8d8 100644 --- a/src/namestore/namestore.h +++ b/src/namestore/namestore.h @@ -437,6 +437,16 @@ struct RecordRemoveMessage */ #define RECORD_REMOVE_RESULT_FAILED_TO_REMOVE 5 +/** + * Internal error, failed to access database + */ +#define RECORD_REMOVE_RESULT_FAILED_ACCESS_DATABASE 6 + +/** + * Internal error, failed to access database + */ +#define RECORD_REMOVE_RESULT_FAILED_INTERNAL_ERROR 7 + /** * Remove a record from the namestore response diff --git a/src/namestore/namestore_api.c b/src/namestore/namestore_api.c index f48ea25e5..3b9675e25 100644 --- a/src/namestore/namestore_api.c +++ b/src/namestore/namestore_api.c @@ -422,6 +422,14 @@ handle_record_remove_response (struct GNUNET_NAMESTORE_QueueEntry *qe, ret = GNUNET_SYSERR; emsg = _("Failed to remove records from database"); break; + case RECORD_REMOVE_RESULT_FAILED_ACCESS_DATABASE: + ret = GNUNET_SYSERR; + emsg = _("Failed to access database"); + break; + case RECORD_REMOVE_RESULT_FAILED_INTERNAL_ERROR: + ret = GNUNET_SYSERR; + emsg = _("unknown internal error in namestore"); + break; default: GNUNET_break (0); ret = GNUNET_SYSERR; diff --git a/src/namestore/plugin_namestore_postgres.c b/src/namestore/plugin_namestore_postgres.c index 0df6aa413..214727f7c 100644 --- a/src/namestore/plugin_namestore_postgres.c +++ b/src/namestore/plugin_namestore_postgres.c @@ -370,6 +370,7 @@ namestore_postgres_put_records (void *cls, * @param iter iterator to call with the result * @param iter_cls closure for 'iter' * @return GNUNET_OK on success, GNUNET_NO if there were no results, GNUNET_SYSERR on error + * 'iter' will have been called unless the return value is 'GNUNET_SYSERR' */ static int get_record_and_call_iterator (struct Plugin *plugin, @@ -466,6 +467,7 @@ get_record_and_call_iterator (struct Plugin *plugin, * @param iter function to call with the result * @param iter_cls closure for iter * @return GNUNET_OK on success, GNUNET_NO if there were no results, GNUNET_SYSERR on error + * 'iter' will have been called unless the return value is 'GNUNET_SYSERR' */ static int namestore_postgres_iterate_records (void *cls, @@ -544,6 +546,7 @@ namestore_postgres_iterate_records (void *cls, * @param iter function to call with the result * @param iter_cls closure for iter * @return GNUNET_OK on success, GNUNET_NO if there were no results, GNUNET_SYSERR on error + * 'iter' will have been called unless the return value is 'GNUNET_SYSERR' */ static int namestore_postgres_zone_to_name (void *cls, diff --git a/src/namestore/plugin_namestore_sqlite.c b/src/namestore/plugin_namestore_sqlite.c index b3f57033a..8b35fb84c 100644 --- a/src/namestore/plugin_namestore_sqlite.c +++ b/src/namestore/plugin_namestore_sqlite.c @@ -752,7 +752,6 @@ namestore_sqlite_zone_to_name (void *cls, "sqlite3_reset"); return GNUNET_SYSERR; } - return get_record_and_call_iterator (plugin, stmt, iter, iter_cls); } -- 2.25.1