Re: revamp sdt.h

[Roland McGrath <roland@redhat.com>]

Hi Stan,

I was in a lousy mood on Friday, so sorry for the tone of my message.

My review of your changes had objections to some parts thoroughly isolated
to the tests, or the headers, and no objections to the translator changes.
For a variety of reasons like that, I find it far preferable always to do
multiple small commits rather than individual large ones.  Of course, it
always helps collaboration to push your branch frequently while you work,
and doing smaller-grained commits makes that easier.

Whenever you would use two separate paragraphs in traditional ChangeLog
format (i.e. a blank line between two * lines), then those should be two
separate commits.  Those should be separate whenever one change is
separable from the other (and sometimes even when they aren't really).

For example, I would have done one commit of just the systemtap.exp change,
one of just the sdt.h change, and another with the translator changes.

This makes it easier to revert one part cleanly (or just avoid merging it).
When reviewing changes, it makes it easier to approve/reject one part, etc.

Doing reversions by hand almost always results in unintended changes, and
one really never knows until much later whether they were in fact as
inconsequential as they seemed.

> > Your addition of _SDT_SEMAPHORE... I thought we had abandoned the semaphore thing,
> 
> The implementation is unchanged;  I changed the name to match the pattern in 
> sdt.h.  Nothing sacred about the name or implementation;  we can certainly 
> change it.  The semaphore mechanism is used to implement the *_ENABLED 
> mechanism, the usefulness of which is described by Mark here:
>   http://sourceware.org/ml/systemtap/2009-q1/msg00959.html

I don't really have an opinion about the semaphore feature.  It's just that
in my long posting proposing what we're now calling "sdt v3", I said I was
leaving out the semaphore because it wasn't being used, and nobody followed
up to say otherwise.  I had expected others to participate with me in the
discussion of my code before anyone just started reworking it.  I spent a
long time making the header file as pretty as it can be, so I'm more
sensitive about willy-nilly changes than is entirely reasonable.


The includes magic changes I made for the tests were done quite carefully,
and I did test them (using runcheck but not installcheck), so I'm surprised
you had issues I didn't see.  It's all rather subtle, so I think it's wise
to discuss any such changes rather than just sweep them in.  (I suppose I
should have done so.)  And, frankly, for anything that even might either be
subtle or ever be important, commits with a log explanation of "Tweak it"
are just inadequate.

Those paths need to be exactly just right to have installcheck actually
test the installed headers so we can know they got installed properly,
which is its purpose.  For the runcheck case, they need to be differently
exactly just right to have it find the right headers in the build and
source directories and no others.  For both cases, it's important that they
be found in an -isystem path so that -pedantic doesn't emit meaningless
stupid warnings that there is no other way to suppress (until GCC is fixed).


Thanks,
Roland