This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH v5] Add pretty printers for the NPTL lock types
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Martin Galvan <martin dot galvan at tallertechnologies 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 14:30:57 -0700 (PDT)
- 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>
> Funny, that's the way I originally had it but Sid asked me to move
> everything to a top-level subdir. I personally like it better this
> way, especially after I introduced the test suite.
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.
> > We don't normally use _ in file names in libc, we only use -.
> > Why are some of the new files named with _s?
> AFAIK Python requires it:
> >>> import a-b.py
> File "<stdin>", line 1
> import a-b.py
> SyntaxError: invalid syntax
Oy. Such a fine language, I forget sometimes how wonderful it is.
> > 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.
> Really? I though I had fixed them all. Could you point out where?
(re-search-forward "\\. \\S ")
> Yeah, I agree. I can add something like:
> PYTHON := python
> to Makeconfig, but I'm not messing with configure scripts for this
> particular patch.
Actually, the closest analogs we have are things like PERL, which are set
only in configure and config.make.in, and not in Makeconfig. It's not
hard. The code also needs to disable stuff when there is no Python binary
available. But I see we already have benchtests/Makefile doing this
wrongly as your change does. So just leave what you had for now, and then
we'll clean them up together (by which I mean I will shame Siddhesh into
doing it ;-).
> I think $*.pysym eases understanding of that line, and it doesn't
> depend on the order of the prerequisites. But I can change it if you
> want to.
> 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.