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

Batch of checked header changes from CCI 2021-05 #448

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mattmccutchen-cci
Copy link
Contributor

@mattmccutchen-cci mattmccutchen-cci commented May 26, 2021

As previously discussed with Sulekha, CCI is preparing to submit a batch of changes to the checked headers that we made in support of our recent porting efforts. I'm going ahead and submitting a draft PR so that I can more easily refer to some of our changes in issues that I'm about to file. Warning: I may still overwrite the commits of the PR via force-push.

One part of the PR that I believe is in final form and that you can go ahead and review if you wish is the change to make #include-ing a wrapper header an error if the original header does not exist on the system. This change is currently in this commit (though I may overwrite the commit later). It might make sense to make this change into a separate PR (at the cost of some additional overhead) because it is different in nature from all our other changes; let me know if you'd like me to do so.

The corresponding draft PR for the tests in the checkedc-clang repository is microsoft/checkedc-clang#1064.

mattmccutchen-cci and others added 5 commits May 21, 2021 15:23
original header is not available on the system.

As agreed with Sulekha. Conditionalizing the installation via CMake
might be even better, but that would take some more thought, especially
if we still want to test threads_checked.h on systems that don't have
threads.h.

Note that for the wrapper headers (`foo.h` as opposed to
`foo_checked.h`), we reverse the order of the conditionals because we
always want to use the `#error` message from `foo.h`, not the one from
`foo_checked.h`, which wouldn't make sense if the user originally
included `foo.h`.

The `#error` message is subject to change in Microsoft's code review.
This applies the change from
microsoft#309 to support the common use
case for sprintf-like functions with null-terminated arrays.
This is analogous to microsoft#298 for
`free` and presumably is safe for the same reason.
Co-authored-by: John Kastner <john@correctcomputation.com>
Co-authored-by: mwhicks1 <mwh@correctcomputation.com>
Originally by Mike with no stated reason. We're discussing whether we
want to keep it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant