[Crash-utility] struct structname.member1,member2,...

[Dave Anderson]

Olivier Daudel wrote:

> Hi Dave,
> This patch try to extend your technologie in list to struct.
> cmd_struct() is modified.
> cmd_union() and cmd_pointer() should also be modified.

Hi Olivier,

After taking in the "list" patch, I also thought of applying it to
struct, union, and * (pointer), but decided against it for now.
It just simply easier to dump the whole structure, and grep
for individual fields.

In the case of "list -s", it did make sense, because typically
it iterates over a large number of data structures.

>
>
> Do we really have to clone cmd_struct() in these alternatives ?
> Why not make so that cmd_struct(), cmd_union() and cmd_pointer() do call a
> common function accepting some flags?
> Olivier
>
> struct struct_name address | symbol     <- unchanged
> struct struct_name      <-unchanged
> struct struct_name -o      <- unchanged
>

Probably... but I don't want to risk breaking any of them.

>
> New and the main goal
> crash> struct task_struct.pid,tgid f6044560
>   pid = 3533,
>   tgid = 3533,
>

For what it's worth, you can use multiple instances of
the -R option to the "task" function to get the above.

>
> Also new and in the main goal
> crash> ps 1
>    PID    PPID  CPU   TASK    ST  %MEM     VSZ    RSS  COMM
>       1      0   0  f7e8caa0  IN   0.0    1744    644  init
> crash> task_struct.children f7e8caa0
>   children = {
>     next = 0xf7e8c0d0,
>     prev = 0xc1f060d0
>   },
> crash> struct task_struct.pid,tgid -l task_struct.sibling 0xf7e8c0d0
>   pid = 2,
>   tgid = 2,
>

i.e., you can do this:

crash> task -R pid -R tgid
PID: 18460  TASK: c4786000  CPU: 0   COMMAND: "crash"
  pid = 18460,
  tgid = 18460,


>
> New (acceptable side effect)
> crash> struct task_struct.pid,tgid
> struct task_struct {
>    [152] pid_t pid;
> }
> struct task_struct {
>    [156] pid_t tgid;
> }

That's ugly, and it's one of the reasons I decided against
implementing this.

>
>
> struct struc_name array counter   <- unchanged
>
> New (acceptable side effect)
> crash> struct task_struct.pid,tgid f6044560 3
>   pid = 3533,
>   pid = 2850,
>   pid = -167489336,
>   tgid = 3533,
>   tgid = 2850,
>   tgid = -167489412,
>

That's really ugly, because you can't obviously correlate the
fact that the members of the same structure are not dumped
together.  So that's definitely *not* acceptable in my opinion.

The only way I can consider accepting a change of this magnitude
is if the patch does all of this:

(1) addresses all 3 functions. (struct, union, and pointer)
(2) does the comma separated list of members in a structure dump.
    like your "struct task_struct.pid,tgid f6044560" example.
(3) does *not* do the "split" structure output, like your
    "struct task_struct.pid,tgid" example.
(4) if an array counter is added, the members from the same
    structure should be grouped together.
(5) these changes do not break the current functionality.

Your patch only accomplishes #2.

Again, I really don't think that this feature is all that
necessary, given that individual structures are typically
not so big such that it's a burden to show the whole structure
or just grep for what you want.

And I really worry about #5, because in order to accomplish
#1 through #4, some of the current functionality is going to
have to be significantly modified.

In any case, if you can come up with something that does
all 5 of the above, please post it again.  But I'm going to
have to let this one pass for now.

However, if you want to continue this, it may make sense to
create new functions rather than patching the old ones.
Because of the way the old functions work, it's difficult to
coerce them into doing everything that you're proposing,
so it's probably worth writing your own new functions.

Thanks,
  Dave

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20061018/dd753768/attachment.htm>