Skip to content

Conversation

@jsoref
Copy link

@jsoref jsoref commented Oct 18, 2023

This PR was rebased once to pull in recent commits (which added 3 new typos to be fixed). It's sizable, and exceeds some of GitHub's tolerances for PR sizes. I'm happy to split or do various other things to aid in ingesting changes. I will try to comment on the changes I'm proposing, but it'll take a while.

Fixes misspellings identified by the check-spelling action.

The misspellings have been reported at https://github.com/jsoref/google-quiche/actions/runs/6559260454#summary-17814518323

The action reports that the changes in this PR would make it happy: https://github.com/jsoref/google-quiche/actions/runs/6559261206#summary-17814520841

jsoref added 30 commits October 18, 2023 05:21
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
jsoref added 28 commits October 18, 2023 06:42
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Copy link
Author

@jsoref jsoref left a comment

Choose a reason for hiding this comment

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

I'm assuming there are no downstream API consumers that would care about changes. Obviously, if there are, changes that would break them can be dropped.

Fwiw, there were a couple of instances of crypt/cryption which I had hoped could be (de|en)crypt[ion] but I didn't have the energy to determine if one or the other was correct.


// If enabled, parse the available portion of headers even on a
// HEADERS_TOO_LONG error, so that that portion of headers is available to the
// HEADERS_TOO_LONG error, so that portion of headers is available to the
Copy link
Author

Choose a reason for hiding this comment

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

Sometimes that that is meaningful. In this case that portion should work just as well as that that portion even though there's technically a change in the parsing.

"\r\n";
std::string message_body =
"A chunkjed extension \r\n"
"A chunked extension \r\n"
Copy link
Author

Choose a reason for hiding this comment

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

It's possible that something cares about the precise character stream, but I'm hoping there isn't.

// mistakes that the caller does not lower case the headers in keys.
// Better performance can be achieved by asking caller to lower case
// the keys and RemoveAllOfheaderInlist just does lookup.
// the keys and RemoveAllOfheaderInList just does lookup.
Copy link
Author

Choose a reason for hiding this comment

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

There are quite a few cases where comments try to mention function names and don't get them right, you can see based on the context that this was one of them.

if (it == header_lines_.end()) {
QUICHE_DLOG(WARNING)
<< "Attempting to remove last token from a non-existent "
<< "Attempting to remove last token from a nonexistent "
Copy link
Author

Choose a reason for hiding this comment

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

Happy to drop this or anything else -- in general, I group changes by the resulting spelling so if you don't like a correction, you can flag it once and I'll drop the commit.

status = v.ValidateSingleHeader(
"name2",
"Antidisestablishmentariansism is supercalifragilisticexpialodocious.");
"Antidisestablishmentariansism is supercalifragilisticexpealidocious.");
Copy link
Author

Choose a reason for hiding this comment

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

both tokens were present in this repository. There was no obvious explanation for why this incorrect spelling was present.

TEST_F(QuicStreamSequencerBufferTest, ReadvAcrossBlocks) {
std::string source(kBlockSizeBytes + 50, 'a');
// Write 1st block to full and extand 50 bytes to next block.
// Write 1st block to full and extend 50 bytes to next block.
Copy link
Author

Choose a reason for hiding this comment

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

possibly expand

QuicOwnedPacketBuffer(QuicOwnedPacketBuffer&& owned_buffer)
: QuicPacketBuffer(std::move(owned_buffer)) {
// |owned_buffer| does not own a buffer any more.
// |owned_buffer| does not own a buffer anymore.
Copy link
Author

Choose a reason for hiding this comment

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

I used a heuristic which I think was right (e.g. here) more often than it was wrong (there's one place below where I got it wrong, which I'll fix after I finish leaving these notes -- sorry)
https://www.grammarly.com/blog/anymore-vs-any-more/

// interface_index.
//
// additional_attributes are RTAs (man 7 rtnelink) that will be sent together
// additional_attributes are RTAs (man 7 rtnetlink) that will be sent together
Copy link
Author

Choose a reason for hiding this comment

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

sometimes there are man pages I don't recognize (especially in the network section), but this just appears to be a typo.

// decryption so that the crypters latch when expected. The crypters are in
// |dest_conn|, but we don't want to try and use them there. Instead we swap
// them into |framer|, perform the decryption with them, and then swap ther
// them into |framer|, perform the decryption with them, and then swap them
Copy link
Author

Choose a reason for hiding this comment

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

Neither there nor their seemed to work, so...


// Check that the framer stops delivering header data chunks once the visitor
// declares it doesn't want any more. This is important to guard against
// declares it doesn't want anymore. This is important to guard against
Copy link
Author

Choose a reason for hiding this comment

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

This is wrong, sorry.

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.

1 participant