Skip to content

Ptrdist/anagram: Checked Strings #79

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

Merged
merged 4 commits into from
Nov 16, 2017

Conversation

lenary
Copy link
Collaborator

@lenary lenary commented Nov 15, 2017

This depends on checkedc/checkedc#239

The good news is the only _Unchecked parts left are:

  • printf, fprintf
  • setjmp, longjmp

Inserted some dynamic bounds cast to cast from an array_ptr to a nt_array_ptr. This could be wrong, please make sure it's the right approach. The bounds in this file are all whacky, we should be using nt_array_ptr in far more places and i will revisit it after I've finished making everything else checked.

@lenary lenary requested a review from dtarditi November 15, 2017 03:56
@lenary
Copy link
Collaborator Author

lenary commented Nov 16, 2017

@dtarditi this is waiting on a review.

@lenary
Copy link
Collaborator Author

lenary commented Nov 16, 2017

This is ready for review again, now includes unchecked blocks.

@lenary
Copy link
Collaborator Author

lenary commented Nov 16, 2017

Oh this may not build immediately, as hacks.h was in the other branch. Once merged it will.

@@ -661,12 +661,11 @@ _Unchecked int Cdecl main(int cpchArgc, _Array_ptr<char*> ppchArgv : count(cpchA

while (GetPhrase(&achPhrase[0], sizeof(achPhrase)) != NULL) {
if (isdigit(achPhrase[0])) {
cchMinLength = atoi((char*)achPhrase);
cchMinLength = atoi((_Nt_array_ptr<char>)achPhrase);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cast should be wrapped in an unchecked block: casts from _Array_ptr to _Nt_array_ptr won't be allowed. The compiler work to add this checking is tracked by the existing work item checkedc/checkedc-clang#391.

Copy link
Contributor

@dtarditi dtarditi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggested change. Otherwise - looks good.

void DumpWords(void) {
static int X;
int i;
X = (X+1) & 1023;
if (X != 0) return;
for (i = 0; i < cpwLast; i++) _Unchecked{ wprint((char*)apwSol[i]->pchWord); }
for (i = 0; i < cpwLast; i++) _Unchecked { wprint((_Nt_array_ptr<char>)apwSol[i]->pchWord); }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this won't be either

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it has an unchecked block because we couldn't prove the bounds anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's why I didn't mention it.

@lenary lenary merged commit 506df55 into microsoft:master Nov 16, 2017
@lenary lenary deleted the nt_strings_ptrdist_anagram branch November 16, 2017 23:42
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.

2 participants