Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

complete review and testing process from original illumos-extra PR #1

Open
jclulow opened this issue Aug 28, 2020 · 0 comments
Open

Comments

@jclulow
Copy link
Member

jclulow commented Aug 28, 2020

Joyent never quite got around to completing review and testing on TritonDataCenter/illumos-extra#12 (submitted by @richlowe), which forms the basis of this repository. We should do that. Rich's comments reproduced below:

richlowe commented on Sep 21, 2013:

This contains a chunk of fixes to improve compatible with Sun cpp
(or correctness in general). It started out as a fix for
TritonDataCenter/smartos-live#171 but grew as more problems became apparent.

There are still at least 2 issues outstanding:

  1. As documented in the first non-HACK commit, our treatment of
    spaces in macros is, while considerably better, still not
    strictly compatible in a couple of ways that might be
    important

  2. #if conditions are not macro-expanded prior to being
    processed, such that #if SOME_MACRO(5) is a syntax error.
    This is proving particularly frustrating to fix.

This will definitely require more testing on the SmartOS side to
be really sure it's safe, I'd recommend a complete platform build,
testing of any SDC bits that use dtrace -C, at least. I'd also
probably suggest running test.sh | squash.py over a SmartOS
/usr/include and checking for anything that would affect the
parseability of the result. (this is why I left them in the
source).

richlowe commented on Oct 7, 2013:

I'm more concerned with review than testing. Making sure SDC isn't
broken, etc, is important, but the scripts I attached and their
results (and just how plainly wrong some of the prior behaviour
was) suggest it's pretty unlikely. The major concern I have is
that the code is so interestingly structured (because here on the
11/34, we don't have much memory to play with!), that I could have
screwed it entirely and have it still just about work right :\

I think that fixing cpp like this is probably the right thing to
do short-term, but I pretty firmly believe that -- given just how
broken this cpp is and was -- that switching to an ANSI cpp while
perhaps wrong is actually not any worse. I think the damage to
dtrace -C would be real, but minimal (and probably defensible).
The real problem would be how much pain it caused as.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant