This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Oops. Forgot to actually attach patch. On Fri, Aug 24, 2012 at 3:00 PM, Aaron Gamble <agamble@google.com> wrote: > New patch. See inline comments. > > (FYI today is the last day of my internship, so this e-mail address > will be dead very soon. I can be reached at invalidfunction or > jaarongamble both at gmail.com) > > gdb/ChangeLog: > * data-directory/Makefile.in: Add gdb/thread_printing.py. > * doc/gdb.texinfo: Add section about thread name printers in Python section. > Add reference to thread name printers in 'info threads' section. > * python/lib/gdb/__init__.py: Import ThreadNamePrinter. > * python/lib/gdb/thread_printing.py: new file. > (class ThreadNamePrinter): base class, contains helper functions > and list of registered printers. > (class ExamplePrinter): example printer class for reference. > (class EnableThreadNamePrinter, DisableThreadNamePrinter, > InfoThreadNamePrinter): > gdb.commands for managing and getting information about thread > name printers. > (register_thread_name_printer_commands): Registers thread name > printer commands. > * python/py-inferior.c (add_thread_object): Save pointer to > thread_object in thread_info. > * python/py-infthread.c (prepare_thread_name_printers): Prepares > all the registered thread name printers > and returns a list of the printers. Called once by > thread.c:print_thread_info. > (thread_printer_enabled): Returns TRUE if printer enabled. > (apply_thread_name_pretty_printers): Iterates through the list of > printers calling print_thread until one returns a string. > Called for each thread by thread.c:print_thread_info > * python/python-internal.h: Extern gdbpy_print_thread_cst PyObject > string object. > * pyhon/python.c: Declare and assign gdbpy_print_thread_cst. > * testsuite/gdb.python/Makefile.in: add py-threadprettyprint. > * testsuite/gdb.python/py-threadprettyprint.c: Basic threaded > program for testing thread name printing. > * testsuite/gdb.python/py-threadprettyprint.exp: Tests for thread > name printers. > * thread.c (print_thread_info): Calls prepare_thread_name_printers > to initialize printers and get list of printers. > Calls apply_thread_name_pretty_printers for each thread if no > name for thread is already stored. > (_initialize_thread): Add aliases for 'info threads' to > disambiguate between 'info thread-name-printer' > > > On Thu, Aug 23, 2012 at 9:17 AM, Tom Tromey <tromey@redhat.com> wrote: >> >>>>>>> "Aaron" == Aaron Gamble <agamble@google.com> writes: >> >> Tom> How about just 'gdb.threads.add_printer'? >> Tom> Or just have the base class constructor do it, and make it private? >> >> Aaron> How about gdb.thread_printing.add_printer? It also seems fine to me to >> Aaron> have a method in the base class. >> Aaron> e.g. >> Aaron> myclass().add_printer("my printer") >> Aaron> # instantiates class and adds it to list of printers with alias "my printer" >> >> It's sort of a norm in gdb to have the instantiation of the class >> install the CLI bits as a side effect. E.g., Command and Parameter do >> this. I don't insist on it, though. > I just stuck with myclass().add_printer("my alias") for now. >> >> I like the name gdb.threads over gdb.thread_printing, just because I >> assume we'll want to have more thread utility code, and this would >> provide a spot to put it. > Done. changed to gdb.threads. > > >> Tom> However, if you do this, then you probably ought to change how thread >> Tom> object lifetimes are handled more generally, getting rid of >> Tom> threadlist_entry, etc. >> >> Aaron> Not entirely sure what you mean here, but I will look into it. My >> Aaron> unfamiliarity with the codebase is to blame. >> >> No worries. >> >> The current code is written in a way to let us associate a gdb.Thread >> object with a thread_info in an indirect way. However, this probably >> doesn't perform extremely well if there are many threads. >> >> So, your approach is superior. However, I think it is best to do the >> conversion completely. This could be a separate refactoring patch -- >> just rip out the old threadlist_entry stuff and replace it with your >> approach. You can see how we handled this in breakpoint.h (search for >> struct breakpoint_object) to make it work without including python.h >> everywhere. > I will leave this for dje@ to do. > > >> Tom> What is the rationale for the 'prepare' step, anyway? >> >> Aaron> This is for the printers to do any one time setup before printing a >> Aaron> list of threads. A common case I can see is if the printer needs to >> Aaron> examine memory and traverse something like a linked list. Without a >> Aaron> call like this, or an indicator in print_thread, there is no way for a >> Aaron> printer to know the different between multiple calls to info threads. >> >> Ok, I see. >> Should there also be an "unprepare" step so that these objects can >> release resources after "info threads" exits? > That's a good point. I've added a cleanup step. > >> Aaron> Ah, sorry, I'm unfamiliar with the Python C api. I will add a call to >> Aaron> gdbpy_print_stack when printers == NULL. Are you also worried about >> Aaron> the potential exceptions raised in PyList_Check and PyList_Size? I >> Aaron> suppose in that case just having a call to gdbpy_print_stack at the >> Aaron> end of this function or in each case of printers == NULL would be >> Aaron> sufficient. >> >> Nearly all Python functions have to have their result checked for error. >> This often leads to spaghetti code, unfortunately, but that's how it is. >> >> Exactly how to handle the error depends on the situation. >> >> In "Python-facing" code, the typical thing to do is propagate the error >> -- release locally-acquired resources and return NULL (or -1 or whatever >> it is for the current context). >> >> In "gdb-facing" code, usually we call gdbpy_print_stack. This isn't >> ideal, since in many situations I think it would be preferable to >> convert the Python exception to a gdb exception. >> >> In your particular case I think it is friendliest to the user to call >> gdbpy_print_stack, even assuming we implement real exception handling, >> just because this approach means that some bad Python code won't make >> "info threads" fail. > Added stack trace printing to "handle" exceptions. > >> Tom> If you're making a list of printers in the .py code, you might as well >> Tom> filter there instead of having this. It's a lot easier to do it there. >> >> Aaron> The thing is that we do not create a new list. The same list is >> Aaron> returned instead. I suppose if we add the check to >> Aaron> prepare_thread_name_printers and just return a new list, that would be >> Aaron> fine as well. >> >> I got a little lost here. >> >> If there is a single list, where disabled items are filtered when >> iterating over it, then don't bother returning a list at all, just use >> the global one from the .py code. >> >> Aaron> The problem with only setting names on thread-creation events is that >> Aaron> a library managing threads and assigning internal thread names we may >> Aaron> wish to print will potentially not have set the name at the time the >> Aaron> thread is created. >> >> Ok, makes sense. >> >> Aaron> e.g. >> Aaron> 1. thread created. >> Aaron> 2. thread-created event in gdb. (no name available yet) >> Aaron> 3. library sets internal name for thread. >> >> In this scenario I was picturing that the python code would set a >> breakpoint to capture the interesting event. But I can accept that this >> may not always be desirable. >> >> Aaron> The thread name is assigned here because I think it is safe to assume >> Aaron> that once a thread has a name, that name will not change. Also, if a >> Aaron> user assigns a name via 'thread name foo', they would want that name >> Aaron> to override any thread name printer. >> >> Ok, thanks. >> >> Aaron> Ok. I'll use a global static variable for the list of printers in >> Aaron> python/py-infthread.c and use dummy functions for the #ifdef stuff. >> >> IIUC it could all just be in the python code. > You are entirely right. I changed this to just keep the printer in the > Python world, along with some other changes that cleans things up.
Attachment:
thread_name_printers_2.patch
Description: Binary data
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |