View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0007625 | GNUnet | set service | public | 2023-01-29 21:18 | 2023-02-08 13:13 |
Reporter | ulfvonbelow | Assigned To | Christian Grothoff | ||
Priority | normal | Severity | trivial | Reproducibility | always |
Status | assigned | Resolution | open | ||
Product Version | Git master | ||||
Summary | 0007625: Operation->inquiries_sent is full of meaningless values | ||||
Description | The only place anything is inserted into this multihashmap is in decode_and_send and handle_union_pdp_inquiry in gnunet-service-setu.c. What is inserted is the address of a stack-allocated variable that promptly goes out of scope. | ||||
Steps To Reproduce | Read gnunet-service-setu.c. Be confused. | ||||
Additional Information | Not an actual bug, since these values are never actually used. Still, confusing, and could lead to bugs in the future. Patch attached to stuff the enum value in question into a (void *) instead of storing a meaningless address. | ||||
Tags | patch | ||||
Attached Files | 0001-SETU-put-a-sensible-value-in-inquiries_sent.patch (1,998 bytes)
From 17afe7b3d95a4befc769f79c271a4a3361e2b067 Mon Sep 17 00:00:00 2001 From: ulfvonbelow <strilen@tilde.club> Date: Sun, 29 Jan 2023 05:39:18 -0600 Subject: [PATCH] SETU: put a sensible value in inquiries_sent. Currently this stuffs the address of an integer-sized stack-allocated variable into inquiries_sent, which will be irrelevant as soon as the stack frame is destroyed. It's a pain to allocate an integer-sized region of memory just to hold a value that's no larger than a pointer, so for now let's use intptr_t to cast the value. Also, none of these values are currently used. --- src/setu/gnunet-service-setu.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/setu/gnunet-service-setu.c b/src/setu/gnunet-service-setu.c index ba32af577..ad81dd28d 100644 --- a/src/setu/gnunet-service-setu.c +++ b/src/setu/gnunet-service-setu.c @@ -37,6 +37,7 @@ #include <gcrypt.h> #include "gnunet_setu_service.h" #include "setu.h" +#include <stdint.h> #define LOG(kind, ...) GNUNET_log_from (kind, "setu", __VA_ARGS__) @@ -2917,7 +2918,7 @@ decode_and_send (struct Operation *op) hashed_key); GNUNET_CONTAINER_multihashmap_put (op->inquiries_sent, hashed_key, - &mcfs, + (void *) (intptr_t) mcfs, GNUNET_CONTAINER_MULTIHASHMAPOPTION_REPLACE ); @@ -3642,7 +3643,7 @@ handle_union_p2p_inquiry (void *cls, hashed_key); GNUNET_CONTAINER_multihashmap_put (op->inquiries_sent, hashed_key, - &mcfs, + (void *) (intptr_t) mcfs, GNUNET_CONTAINER_MULTIHASHMAPOPTION_REPLACE ); -- 2.38.1 | ||||
|
Well, I think the value is actually OK, as the intent was to simply track of the keys. So any non-NULL pointer is fine. But sure, maybe something more sensible could be put in there. I'm more concerned that the map is never *used* (no get/contains). Seems like some logic from the design was omitted in the implementation. |
Date Modified | Username | Field | Change |
---|---|---|---|
2023-01-29 21:18 | ulfvonbelow | New Issue | |
2023-01-29 21:18 | ulfvonbelow | Status | new => assigned |
2023-01-29 21:18 | ulfvonbelow | Assigned To | => Florian Dold |
2023-01-29 21:18 | ulfvonbelow | Tag Attached: patch | |
2023-01-29 21:18 | ulfvonbelow | File Added: 0001-SETU-put-a-sensible-value-in-inquiries_sent.patch | |
2023-02-08 10:29 | schanzen | Assigned To | Florian Dold => Christian Grothoff |
2023-02-08 13:13 | Christian Grothoff | Note Added: 0019773 |