[Crash-utility] [PATCH v2] struct: Fix handing of percpu symbols (when symbolic argument only)

[Aaron Tomlin]

On Thu 2016-09-29 12:37 -0400, Dave Anderson wrote:
[ ... ]
> Given that you are essentially specifying a completely new argument
> type to the struct command, I'm going to have to NAK this patch.

That's OK.

> To do what you want, honestly I don't really feel that it's asking too much
> to make it a two-step process, i.e, get the address first, and then apply it
> to the struct command.  

Agreed.

> However, if you really feel it's worth it, I suppose you could create a 
> new "indirect-pointer-argument-type" for the struct command that explicitly
> specifies that it is a pointer to the target address.

No, it's not worth it actually. Perhaps a warning instead?
For e.g. something like this:

Subject: [PATCH] struct: warn if percpu symbol is of type pointer

---
 symbols.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/symbols.c b/symbols.c
index 8b0d8b9..6447fad 100644
--- a/symbols.c
+++ b/symbols.c
@@ -81,6 +81,7 @@ static void cmd_datatype_common(ulong);
 static void do_datatype_addr(struct datatype_member *, ulong, int,
 			     ulong, char **, int);
 static void process_gdb_output(char *, unsigned, const char *, int);
+static char *expr_type_name(const char *);
 static int display_per_cpu_info(struct syment *, int, char *);
 static struct load_module *get_module_percpu_sym_owner(struct syment *);
 static int is_percpu_symbol(struct syment *);
@@ -6112,6 +6113,7 @@ cmd_datatype_common(ulong flags)
         char *separator;
         char *structname, *members;
         char *memberlist[MAXARGS];
+        char *typename;
 
         dm = &datatype_member;
 	count = 0xdeadbeef;
@@ -6122,6 +6124,7 @@ cmd_datatype_common(ulong flags)
 	separator = members = NULL;
 	cpuspec = NULL;
 	cpus = NULL;
+	typename = NULL;
 
         while ((c = getopt(argcnt, args, "pxdhfuc:rvol:")) != EOF) {
                 switch (c)
@@ -6245,6 +6248,12 @@ cmd_datatype_common(ulong flags)
 				      sp->name);
 				cpuspec = NULL;
 			}
+			typename = expr_type_name(sp->name);
+			if (cpuspec && typename &&
+				LASTCHAR(typename) == '*' ? 1 : 0)
+				error(WARNING,
+				      "percpu symbol %s is of type pointer.\n",
+				      sp->name);
 	                addr = sp->value;
 			aflag++;
 	        } else {
@@ -6364,6 +6373,9 @@ freebuf:
 
 	if (cpus)
 		FREEBUF(cpus);
+
+	if (typename)
+		FREEBUF(typename);
 }
 
 static void
-- 
2.5.5


-- 
Aaron Tomlin