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

Minor: Fix off-by-one in varint size calculation #111

Merged
merged 1 commit into from
Aug 31, 2023
Merged

Minor: Fix off-by-one in varint size calculation #111

merged 1 commit into from
Aug 31, 2023

Conversation

Palladinium
Copy link
Contributor

I was reading through the varint implementation in postcard to sanity-check one of my own implementations, and I spotted a mistake in the code that calculates the maximum number of varint bytes an integer datatype can take.

The old version adds 7 instead of 6, so instead of the behaviour documented in the comment it would overestimate the number of varint bytes required by one for datatypes whole size is a multiple of 7 bits. Since there are thankfully none among the common integer datatypes, the behaviour is correct both before and after the change.

@netlify
Copy link

netlify bot commented Aug 22, 2023

👷 Deploy Preview for cute-starship-2d9c9b processing.

Name Link
🔨 Latest commit 67a7e74
🔍 Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/64e42e2e7c7d84000883cff9

@netlify
Copy link

netlify bot commented Aug 22, 2023

Deploy Preview for cute-starship-2d9c9b canceled.

Name Link
🔨 Latest commit 67a7e74
🔍 Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/64e42e2e7c7d84000883cff9

@jamesmunns
Copy link
Owner

Thanks, great catch!

@jamesmunns jamesmunns merged commit 16382b9 into jamesmunns:main Aug 31, 2023
4 checks passed
kodiakhq bot referenced this pull request in X-oss-byte/Nextjs Sep 22, 2023
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [postcard](https://togithub.com/jamesmunns/postcard) | workspace.dependencies | patch | `1.0.4` -> `1.0.7` |

---

### Release Notes

<details>
<summary>jamesmunns/postcard (postcard)</summary>

### [`v1.0.7`](https://togithub.com/jamesmunns/postcard/blob/HEAD/CHANGELOG.md#107---Unreleased)

[Compare Source](https://togithub.com/jamesmunns/postcard/compare/v1.0.6...v1.0.7)

### [`v1.0.6`](https://togithub.com/jamesmunns/postcard/blob/HEAD/CHANGELOG.md#106---107)

[Compare Source](https://togithub.com/jamesmunns/postcard/compare/v1.0.5...v1.0.6)

-   Fix off-by-one in varint size calculation by [@&#8203;Palladinium](https://togithub.com/Palladinium) in [https://github.com/jamesmunns/postcard/pull/111](https://togithub.com/jamesmunns/postcard/pull/111)
-   Add specific error for Crc errors by [@&#8203;CBJamo](https://togithub.com/CBJamo) in [https://github.com/jamesmunns/postcard/pull/112](https://togithub.com/jamesmunns/postcard/pull/112)

### [`v1.0.5`](https://togithub.com/jamesmunns/postcard/blob/HEAD/CHANGELOG.md#105---106)

[Compare Source](https://togithub.com/jamesmunns/postcard/compare/v1.0.4...v1.0.5)

-   Add cfg information to docs by [@&#8203;dtolnay](https://togithub.com/dtolnay) in [https://github.com/jamesmunns/postcard/pull/108](https://togithub.com/jamesmunns/postcard/pull/108)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/sammyfilly/Nextjs).
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