This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v5] Add pretty printers for the NPTL lock types
- From: Martin Galvan <martin dot galvan at tallertechnologies dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: libc-alpha at sourceware dot org, Daniel Gutson <daniel dot gutson at tallertechnologies dot com>, "Carlos O'Donell" <carlos at redhat dot com>, Siddhesh Poyarekar <sid at reserved-bit dot com>, Tom Tromey <tom at tromey dot com>
- Date: Mon, 11 Apr 2016 18:55:36 -0300
- Subject: Re: [PATCH v5] Add pretty printers for the NPTL lock types
- Authentication-results: sourceware.org; auth=none
- References: <1460405322-31278-1-git-send-email-martin dot galvan at tallertechnologies dot com> <20160411203830 dot EF45B2C3B22 at topped-with-meat dot com> <CAOKbPbYmtfq=qReDNhTkVgt64cSbsd=+VQ+rHAgigdfbhBdMcQ at mail dot gmail dot com> <20160411213057 dot 92EE82C3B22 at topped-with-meat dot 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:
...
"""