Re: [PING 2][PATCH v3] Add pretty printers for the NPTL lock types

[Siddhesh Poyarekar <sid at reserved-bit dot com>]

Sorry about jumping on this late, I missed this patch on the list
somehow.

The most important requirement IMO is a README in the toplevel for
anyone looking to add pretty printers to other modules in glibc.  What
you have here is the beginnings of a feature set for glibc and we need
to make sure that others can build upon it easily.

> +class Printer(object):
> +    """Printer class which conforms to the gdb pretty printing interface."""
> +
> +    def __init__(self, name):
> +        self.name = name
> +        self.enabled = True
> +        self.subprinters = []
> +
> +    class Subprinter(object):
> +        """A regex-based printer.
> +
> +        Individual pretty-printers are registered as subprinters of a single
> +        Printer instance.
> +        """
> +
> +        def __init__(self, name, regex, callable_obj):
> +            """Initialize a pretty-printer.
> +
> +            Args:
> +                name: The name of the printer.
> +                regex: A regular expression.  When gdb tries to print a
> +                    variable whose type matches the regex, it'll trigger
> +                    this printer.
> +                callable_obj: A function or callable object that gdb will call
> +                    when trying to print some value.  It should return a
> +                    pretty-printer.
> +            """
> +            self.name = name
> +            self.regex = re.compile(regex)
> +            self.callable = callable_obj
> +            self.enabled = True
> +
> +    def add_subprinter(self, name, regex, callable_obj):
> +        """Register a regex-based subprinter."""
> +
> +        self.subprinters.append(self.Subprinter(name, regex, callable_obj))
> +
> +    def __call__(self, value):
> +        """gdb API function.
> +
> +        This is called when trying to print an inferior value from gdb.
> +        If a registered printer's regex matches the value's type, gdb will use
> +        the printer to print the value.
> +        """
> +
> +        type_name = value.type.name
> +
> +        if type_name:
> +            for subprinter in self.subprinters:
> +                if subprinter.enabled and subprinter.regex.match(type_name):
> +                    return subprinter.callable(value)
> +
> +        # Return None if we have no type name or if we can't find a subprinter
> +        # for the given type.
> +        return None
> +
> +def register(objfile):
> +    """Register the pretty printers within the given objfile."""
> +
> +    printer = Printer('Glibc pthread locks')
> +
> +    printer.add_subprinter('pthread_mutex_t', '^pthread_mutex_t$',
> +                           MutexPrinter)
> +    printer.add_subprinter('pthread_mutexattr_t', '^pthread_mutexattr_t$',
> +                           MutexAttributesPrinter)
> +    printer.add_subprinter('pthread_cond_t', '^pthread_cond_t$',
> +                           ConditionVariablePrinter)
> +    printer.add_subprinter('pthread_condattr_t', '^pthread_condattr_t$',
> +                           ConditionVariableAttributesPrinter)
> +    printer.add_subprinter('pthread_rwlock_t', '^pthread_rwlock_t$',
> +                           RWLockPrinter)
> +    printer.add_subprinter('pthread_rwlockattr_t', '^pthread_rwlockattr_t$',
> +                           RWLockAttributesPrinter)
> +
> +    gdb.printing.register_pretty_printer(objfile, printer)
> +
> +register(gdb.current_objfile())

These bits look generic, so I would suggest pulling them out into a
separate file and make it reusable for other modules' pretty
printers. In fact, I think the pretty printing code should go into a
separate directory (gdb-pp or similar, maybe put the README file in
this) so that all pretty printers stay in one place.  It would be
clumsy to have them in different module directories now and then
putting them all into the same directory during installation.  It
would be much easier to test them with all of them in the same
directory.

Siddhesh