----- Original Message ----- > Hello Dave, > > After some many discussions, I rewrite the code to an extension module. > What I want is not only the member of structure page, but also other > structure's members. So for the speed of analysing a big number of data > and not obeying your attitude towards the change of struct command, I > made it an extension module. Thank you -- I do appreciate that. I will post "pstruct.c" on the extensions web page when crash-6.0.7 is released, and will note the "crash-6.0.7 or later" restriction in the "comments" section of its entry. > And about the patch which changes gdb, I create a new method > print_command_2 to do the work. That addition looks fine to me, and does not affect any other commands. Queued for crash-6.0.7. Also, I should note that I have incorporated your "ipcs" command into crash-6.0.7. It is essentially the same as your extension module, with these exceptions: (1) I have re-written the "help" page (2) I replaced your default usage of the nsproxy of the CURRENT_TASK() with that of pid 0, because the command fails if the current task is exiting. (3) Similarly to the "mount" command, I have added a "-n [pid|task]" namespace option: For kernels supporting namespaces, the -n option may be used to display the IPC facilities with respect to the namespace of a specified task: -n pid a process PID. -n task a hexadecimal task_struct pointer. So congratulations are in order -- it is the first new command since 2001! Thanks, Dave > > At 2012-4-19 20:50, Dave Anderson wrote: > > > > > > ----- Original Message ----- > >> At 2012-4-18 21:21, Dave Anderson wrote: > >>> In our original discussions, i thought that I had made it clear > >>> that > >>> the introduction of a new option paradigm with submembers could > >>> be > >>> avoided by using, for example, "page._mapcounter" instead of > >>> having > >>> to enter "page._mapcount.counter"? This option makes the struct > >>> command seemingly violate its own rules, and really confuses > >>> things. > >>> For example, with your patch, a user would see things like this: > >> > >> The most important reason why I insisted this option is the > >> performance. > >> Both original struct and print command are very slow. When kernel > >> debugger wants to parse a bit amount of data, the performance of > >> original struct and print command is not ideal. > >> > >>> > >>> crash> page._mapcount.counter ffffea0000000508 -s > >>> -1 > >>> crash> page._mapcount.counter ffffea0000000508 > >>> struct: invalid format: page._mapcount.counter > >>> crash> page._mapcount ffffea0000000508 > >>> _mapcount = { > >>> counter = -1 > >>> } > >>> crash> page._mapcount ffffea0000000508 -s > >>> struct: invalid data structure reference page._mapcount > >>> crash> > >> > >> An idea of solving this confusion is changing the error > >> information. > >> When users uses "-s" option and error happens, error information > >> suggests to use struct command without "-s" option if it is valid. > >> And vice versa, when error happens without specified "-s" option. > > > > It's not so much the error message wording, it's the usage of a > > completely different option-expression. And you can still display > > the -s information without the extra submember. > > > >>> > >>> I had suggested that you look into the get_member_data() function > >>> in to the gdb/symtab.c file to access the member offset and size > >>> values. > >> > >> Actually, the function need to be changed a lot to support what I > >> want. > >> I need the information of submember, and I need the position and > >> size of > >> bitfield. After investigation, function print_command_1 hides the > >> data > >> that I want. I know it is not a good idea of modifying this > >> function. > >> But what if a new function which has the similar mechanism with > >> function > >> print_command_1? > > > > Right, that's exactly what I suggested below: > > > >>> I also don't like the idea of modifying the prototype of > >>> the stalwart print_command_1() gdb function, and the creation > >>> of a new gdb command. Whenever there is a need to update the > >>> embedded gdb version, patches like this can be problematic. > >>> I would prefer that you create a new "GNU_XXXX" #define, > >>> similar to GNU_GET_SYMBOL_TYPE, pass the request through > >>> the gdb_command_funnel switch statement, and write a new > >>> standalone function to accomplish what you have done in the > >>> print_command_1() function. > > > > Dave > > > > -- > > Crash-utility mailing list > > Crash-utility@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > > > > -- > -- > Regards > Qiao Nuohan > > > > -- > Crash-utility mailing list > Crash-utility@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility