[RFC PATCH 15/15] Ask some questions in comments

[Ákos Uzonyi]

---
 btree.h              |  8 ++++++++
 pidns.c              | 20 ++++++++++++++++++++
 tests/inject-nf.test |  6 ++++++
 3 files changed, 34 insertions(+)

diff --git a/btree.h b/btree.h
index bd416c87..0aa21afe 100644
--- a/btree.h
+++ b/btree.h
@@ -39,6 +39,14 @@ enum btree_iterate_flags {
  */
 struct btree {
 	uint64_t set_value;         /**< Default set value */
+
+	/*
+	Would it make sense to make this pointer type of void**?
+	This type is quite confusing anyway, but maybe void** is a bit more clear.
+
+	As I see we always cast it to uint64_t* so we use it as a pointer to just
+	raw data, but for pointer arithmetic we need to point it to a pointer type.
+	*/
 	uint64_t **data;
 	uint8_t item_size_lg;       /**< Item size log2, in bits, 0..6. */
 	/** Pointer block size log2, in pointers sizes. 8-14, usually. */
diff --git a/pidns.c b/pidns.c
index 9433bf47..a3029995 100644
--- a/pidns.c
+++ b/pidns.c
@@ -18,6 +18,10 @@
 #include "nsfs.h"
 #include "xmalloc.h"
 
+
+/*
+Can we check in any efficient way, that these two btree caches are valid?
+*/
 /* key - NS ID, value - parent NS ID. */
 // struct btree *ns_hierarchy;
 /*
@@ -50,6 +54,17 @@ static const struct {
 
 struct proc_data {
 	int proc_pid;
+
+	/*
+	Does it make sense to cache these values below?
+
+	As far as i know there is no other way to check this cache validity,
+	than re-reading these values from /proc. But then we have to read these
+	values again anyway, so no reason to cache.
+
+	The only thing I find useful to cache is proc_pid.
+	With a cached proc_pid we don't have to read all entries in proc, just one.
+	*/
 	short ns_count;
 	short refcount;
 	uint64_t ns_hierarchy[MAX_NS_DEPTH]; /* from bottom to top of NS hierarchy */
@@ -78,6 +93,11 @@ pid_to_str(pid_t pid)
 	return buf;
 }
 
+/*
+Do we need "last" parameter? Only 0 and our_ns are passed to it, but neither
+does anything useful, as we can't go higher than our_ns anyway
+(ioctl fails with EPERM).
+*/
 /**
  * Returns a list of PID NS IDs for the specified PID.
  *
diff --git a/tests/inject-nf.test b/tests/inject-nf.test
index cadb5adb..9b01345e 100755
--- a/tests/inject-nf.test
+++ b/tests/inject-nf.test
@@ -34,6 +34,12 @@ test_rval()
 test_rval 0
 test_rval 1
 test_rval 0x7fffffff
+
+#This test fails (overflow) with these large values,
+#as we use signed integers in pid translation code
+#(PID_MAX_LIMIT is 2^22, so that seems reasonable)
+#
+#Maybe we could use a different syscall, for example getuid?
 test_rval 0x80000000
 test_rval 0xfffff000
 test_rval 0xfffffffe
-- 
2.26.2