Re: [Crash-utility] [PATCH] add -s option for struct command

----- 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(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility