Re: [PATCH v5] Add pretty printers for the NPTL lock types

[Martin Galvan <martin.galvan@tallertechnologies.com>]

On Mon, Apr 11, 2016 at 6:30 PM, Roland McGrath <roland@hack.frob.com> wrote:
> I'm sorry I didn't participate in the earlier review threads.  I don't know
> what Siddhesh's rationale for that was.  But it's just not the modular
> thing to do.  Not every configuration includes every subdir, so things just
> break if one subdir always assumes that others exist.

I thought Depend took care of that. In any case, Sid remarked that It
would be clumsy to have the printers in different module directories
now and then putting them all into the same directory during
installation. It also makes sense to have them tested separately
through a single Makefile, and it's the right place to put things like
test_common.py and the pretty printers README.

>> > I noticed several lines (perhaps all comments) that were a bit too long.
>> > No line should have column 80 occupied.
>>
>> We discussed this with Sid, here:
>
> Whatever you discussed before, this is a straightforward part of the glibc
> coding style and you don't get to ignore it for new code.

It's a technical issue, actually, and it's explained in all the test
programs. Basically, I saw that having 'if' conditions split in more
than one line seems to confuse gdb to the point where we need to issue
a (somewhat random) number of 'next' commands to advance. This
complicates testing quite a bit.

>> Really? I though I had fixed them all. Could you point out where?
>
> (re-search-forward "\\. \\S ")

Found a couple, will fix them.

>> All my files have a descriptive comment, though it's after the
>> copyright notice. I can move it up if you want to.
>
> That is a nonnegotiable standard for new files added to the source tree.

Ok. Is it ok if I place a single line of comment above the license,
then leave a more detailed explanation below it? E.g. for
nptl-printers.py we'd have:

# Pretty printers for the NPTL lock types.
# Copyright (C) 2016 Free Software Foundation, Inc.
...

"""This file contains the gdb pretty printers for the following types:
...
"""