From 4bf153fdb0d9f8597a2be7791aa9f9fe957b37bc Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Mon, 20 Jan 2020 00:21:50 +0100 Subject: resolve fixmes --- src/exchange/taler-exchange-aggregator.c | 17 ++++------------- src/exchange/test_taler_exchange_httpd_afl.sh | 5 +++-- 2 files changed, 7 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/exchange/taler-exchange-aggregator.c b/src/exchange/taler-exchange-aggregator.c index 8c01213d8..a639b19de 100644 --- a/src/exchange/taler-exchange-aggregator.c +++ b/src/exchange/taler-exchange-aggregator.c @@ -742,9 +742,6 @@ deposit_cb (void *cls, amount_with_fee, deposit_fee)) { - // FIXME(dold): Shouldn't we somehow survive this? Of course it should show up in the auditor's report, - // but due to some misconfiguration with restarts in between deposit fees could have changed. - // That's bad, but should we really refuse to continue completely? GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "Fatally malformed record at row %llu over %s\n", (unsigned long long) row_id, @@ -1283,6 +1280,7 @@ run_reserve_closures (void *cls) now, &expired_reserve_cb, &erc); + GNUNET_assert (1 >= qs); switch (qs) { case GNUNET_DB_STATUS_HARD_ERROR: @@ -1314,8 +1312,6 @@ run_reserve_closures (void *cls) task = GNUNET_SCHEDULER_add_now (&run_reserve_closures, NULL); return; - // FIXME(dold): shouldn't we at least GNUNET_break on the case where - // we have more than one result? Not clear that get_expired reserves can't return more results? } } @@ -1329,8 +1325,7 @@ run_reserve_closures (void *cls) static void run_aggregation (void *cls) { - // FIXME(dold): This should be unsigned, as behavior on signed overflow is undefined ;-) - static int swap; + static unsigned int swap; struct TALER_EXCHANGEDB_Session *session; enum GNUNET_DB_QueryStatus qs; const struct GNUNET_SCHEDULER_TaskContext *tc; @@ -1774,8 +1769,7 @@ wire_prepare_cb (void *cls, NULL); if (NULL == wpd->eh) { - // FIXME(dold): Comment below is a fixme, but without a marker! - GNUNET_break (0); /* why? how to best recover? */ + GNUNET_break (0); /* Irrecoverable */ db_plugin->rollback (db_plugin->cls, wpd->session); global_ret = GNUNET_SYSERR; @@ -1832,10 +1826,8 @@ run_transfers (void *cls) session, &wire_prepare_cb, NULL); - // FIXME(dold): comment below is misleading, should't that be wire_confirm_cb, - // as wire_prepare_cb is already called? if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs) - return; /* continues in #wire_prepare_cb() */ + return; /* continued via continuation set in #wire_prepare_cb() */ db_plugin->rollback (db_plugin->cls, session); GNUNET_free (wpd); @@ -1905,7 +1897,6 @@ run (void *cls, global_ret = 1; return; } - // FIXME(dold): why don't we call this after 'rc' is initialized? Doesn't matter to much but ... ctx = GNUNET_CURL_init (&GNUNET_CURL_gnunet_scheduler_reschedule, &rc); rc = GNUNET_CURL_gnunet_rc_create (ctx); diff --git a/src/exchange/test_taler_exchange_httpd_afl.sh b/src/exchange/test_taler_exchange_httpd_afl.sh index a5d4fcbb8..abfaf0b2f 100644 --- a/src/exchange/test_taler_exchange_httpd_afl.sh +++ b/src/exchange/test_taler_exchange_httpd_afl.sh @@ -25,8 +25,9 @@ # 1) Capture all TCP traffic from 'test-auditor.sh' # 2) Use 'tcpflow -e http -r $PCAP -o $OUTPUT' to get the HTTP streams # 3) Remove HTTP streams unrelated to the exchange as well as the replies -# 4) Compile the exchange with AFL instrumentation -# 5) Run afl-fuzz -i $OUTPUT/ -o afl-tests/ ~/bin/taler-exchange-httpd \ +# 4) Remove duplicated streams (check file size!) +# 5) Compile the exchange with AFL instrumentation +# 6) Run afl-fuzz -i $OUTPUT/ -o afl-tests/ ~/bin/taler-exchange-httpd \ # -c test_taler_exchange_httpd.conf -t 1 -f @@ set -eu -- cgit v1.2.3