Thread (13 messages) 13 messages, 3 authors, 2020-03-17

Re: [PATCH bpf-next 4/4] bpftool: Add struct_ops support

From: Quentin Monnet <hidden>
Date: 2020-03-17 09:25:38
Also in: bpf

2020-03-16 17:57 UTC-0700 ~ Martin KaFai Lau [off-list ref]
On Mon, Mar 16, 2020 at 05:24:52PM -0700, Martin KaFai Lau wrote:
quoted
On Mon, Mar 16, 2020 at 11:54:28AM +0000, Quentin Monnet wrote:
quoted
[...]
quoted
+static int do_unregister(int argc, char **argv)
+{
+	const char *search_type, *search_term;
+	struct res res;
+
+	if (argc != 2)
+		usage();
Or you could reuse the macros in main.h, for more consistency with other
subcommands:

	if (!REQ_ARGS(2))
		return -1;
Thanks for the review!

I prefer to print out "usage();" whenever possible but then "-j" gave
me a 'null' after a json error mesage ...

# bpftool -j struct_ops unregister
{"error":"'unregister' needs at least 2 arguments, 0 found"},null

Then I went without REQ_ARGS(2) which is similar to a few existing
cases like do_dump(), do_updaate()...etc in map.c.

That was my consideration.  However, I can go back to use REQ_ARGS(2)
and return -1 without printing usage.  no strong preference here.
After another look,  I will keep it as is since REQ_ARGS() is a "<"
check.  "argc != 2" is the correct check here.  Otherwise,
allowing 'bpftool struct_ops unregister name cubic dctcp' looks weird.
Ah right, fair enough. I'm good with v2, thanks for the changes!
Quentin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help