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

Deprecate CBOR_CUSTOM_ALLOC #237

Merged
merged 10 commits into from
Dec 29, 2022
Merged

Deprecate CBOR_CUSTOM_ALLOC #237

merged 10 commits into from
Dec 29, 2022

Conversation

PJK
Copy link
Owner

@PJK PJK commented Dec 26, 2022

Description

CBOR_CUSTOM_ALLOC was a mistake, the defaults with CBOR_CUSTOM_ALLOC=ON are compatible with CBOR_CUSTOM_ALLOC=OFF.

I would like to remove it going forward. Will keep this PR open for a few weeks to give people a chance to object.

Pros of having the flag:

  • Clients with simple use cases can (maybe, in practice likely optimized away) save one function resolution indirection

Cons:

  • Complexity everywhere (code, build config, tests, distribution)
  • Possibility of incompatible (esp. shared) libraries

Also reworked the bazel build example since it was miswired and broke due to the flag change

Checklist

  • I have read followed CONTRIBUTING.md
    • I have added tests
    • I have updated the documentation
    • I have updated the CHANGELOG
  • Are there any breaking changes?
    • If yes: I have marked them in the CHANGELOG (example)
  • Does this PR introduce any platform specific code?
  • Security: Does this PR potentially affect security?
  • Performance: Does this PR potentially affect performance?

@codecov-commenter
Copy link

codecov-commenter commented Dec 26, 2022

Codecov Report

Merging #237 (c1032ce) into master (3dfc2d5) will not change coverage.
The diff coverage is 87.50%.

@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   95.78%   95.78%           
=======================================
  Files          20       20           
  Lines        1543     1543           
=======================================
  Hits         1478     1478           
  Misses         65       65           
Impacted Files Coverage Δ
src/cbor/internal/builder_callbacks.c 89.28% <50.00%> (ø)
src/cbor/serialization.c 85.16% <50.00%> (ø)
src/cbor/bytestrings.c 94.64% <75.00%> (ø)
src/cbor/arrays.c 98.24% <100.00%> (ø)
src/cbor/common.c 100.00% <100.00%> (ø)
src/cbor/floats_ctrls.c 96.66% <100.00%> (ø)
src/cbor/internal/memory_utils.c 93.75% <100.00%> (ø)
src/cbor/internal/stack.c 100.00% <100.00%> (ø)
src/cbor/ints.c 99.00% <100.00%> (ø)
src/cbor/maps.c 96.36% <100.00%> (ø)
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@PJK PJK merged commit dfb79c4 into master Dec 29, 2022
chenrui333 added a commit to chenrui333/homebrew-core that referenced this pull request Dec 29, 2022
`CBOR_CUSTOM_ALLOC` got deprecated

relates to PJK/libcbor#237

Signed-off-by: Rui Chen <rui@chenrui.dev>
BrewTestBot pushed a commit to Homebrew/homebrew-core that referenced this pull request Dec 29, 2022
* libcbor 0.10.0
* libcbor: update test
  `CBOR_CUSTOM_ALLOC` got deprecated

  relates to PJK/libcbor#237

Signed-off-by: Rui Chen <rui@chenrui.dev>
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

2 participants