----- Original Message -----
> 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
Fair enough -- but let's avoid the heavy-handed call to expr_type_name()
unless the argument is actually a cpuspec:
--- crash-7.1.5/symbols.c.orig 2016-09-30 09:21:15.577896529 -0400
+++ crash-7.1.5/symbols.c 2016-09-30 09:16:53.676652458 -0400
@@ -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;
@@ -6245,6 +6247,15 @@ cmd_datatype_common(ulong flags)
sp->name);
cpuspec = NULL;
}
+ if (cpuspec) {
+ if ((typename = expr_type_name(sp->name))) {
+ if (LASTCHAR(typename) == '*')
+ error(WARNING,
+ "percpu symbol %s is of type pointer\n",
+ sp->name);
+ FREEBUF(typename);
+ }
+ }
addr = sp->value;
aflag++;
} else {
Dave
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
