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: Martin Galvan <martin dot galvan at tallertechnologies dot com>
- To: Siddhesh Poyarekar <sid at reserved-bit dot com>
- Cc: Roland McGrath <roland at hack dot frob dot com>, libc-alpha at sourceware dot org, Daniel Gutson <daniel dot gutson at tallertechnologies dot com>, "Carlos O'Donell" <carlos at redhat dot com>, Tom Tromey <tom at tromey dot com>
- Date: Tue, 12 Apr 2016 10:19:21 -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> <CAOKbPbausROA=TM1K8Up=ttc9sHxaLtBWUsi7KPxDiS77+Jiig at mail dot gmail dot com> <20160411221656 dot 65CB82C3B38 at topped-with-meat dot com> <20160412030324 dot GE30851 at devel dot intra dot reserved-bit dot com> <20160412055847 dot GF30851 at devel dot intra dot reserved-bit dot com>
On Mon, Apr 11, 2016 at 7:16 PM, Roland McGrath <email@example.com> wrote:
> You can certainly have some common infrastructure code for both the
> pretty-printer implementations and for their tests. It might well be fine
> to have a subdir for that common infrastructure code. But anything
> actually related to a particular type must reside in the subdir responsible
> for the definition of that type.
Does that include the unit tests? That would mean that the
pretty-printers Makefile has to go, since that's what I use for
testing. Perhaps I could try to split its contents between Makerules
and the nptl Makefile? Something like:
test-srcs := $(tests-pretty-printers)
tests-pretty-printers-dest := $(addprefix $(objpfx),$(tests-pretty-printers))
tests-pretty-printers-pp := $(addsuffix -pp,$(tests-pretty-printers-dest))
tests-special += $(tests-pretty-printers-pp)
$(tests-pretty-printers-pp): $(objpfx)%-pp: $(objpfx)% %.py test_common.py
$(test-wrapper-env) $(PYTHON) $*.py $*.c $(objpfx)$*; \
endif # tests-pretty-printers
tests-pretty-printers := test-mutex-attributes test-mutex-printer \
test-condvar-attributes test-condvar-printer \
tests-pretty-printers-libs := $(shared-thread-library)
tests-pretty-printers-libs := $(static-thread-library)
CFLAGS-test-mutex-attributes.c := -O0 -ggdb3 -DIS_IN_build
CFLAGS-test-mutex-printer.c := -O0 -ggdb3 -DIS_IN_build
CFLAGS-test-condvar-attributes.c := -O0 -ggdb3 -DIS_IN_build
CFLAGS-test-condvar-printer.c := -O0 -ggdb3 -DIS_IN_build
CFLAGS-test-rwlock-attributes.c := -O0 -ggdb3 -DIS_IN_build
CFLAGS-test-rwlock-printer.c := -O0 -ggdb3 -DIS_IN_build
But again, I don't know how (or even if) Makerules interacts with
'make check'. I don't know how this will affect cross-testing either.
> This suggests to me that the testing methodology is a poor choice. I'd
> have to review what you've done in more detail to know what I think is the
> best approach. I suspect that using "next" (or "step", etc.) in tests like
> this is just a bad idea altogether (as opposed to only using explicit
> breakpoints). If it turns out that using "next" over an "if" is an
> important thing to be able to do, then put the complex condition into an
> inline or macro.
I don't know if using explicit breakpoints will make any difference,
but I'll give it a shot.
Still, I'd appreciate it if you took the time to properly review the
patch before suggesting such drastic changes. This took me quite a
while to make, so please don't dismiss it just because it has lines
longer than 80 chars. It would also be nice if we listened to what the
previous reviewers have to say about this (Carlos?).
On Tue, Apr 12, 2016 at 12:03 AM, Siddhesh Poyarekar
> So you're only suggesting moving nptl-printers.py to nptl, which seems
> fine given that it is specific to nptl.
If that's what Roland's asking, then fine for me. It's the unit tests
that concern me, since the Makefile does a great deal to run them
correctly. I'd rather *not* touch the Makefile, or mess with the build
> Actually the comments on those lines are quite inane and could be
> dropped altogether, which should take care of most of those long
They're not. I use them to know where to set breakpoints from the test
scripts. It's similar to how testing is done for gdb itself.