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

Add isIpPrefix #45

Merged
merged 6 commits into from
Nov 7, 2023
Merged

Add isIpPrefix #45

merged 6 commits into from
Nov 7, 2023

Conversation

paina
Copy link
Contributor

@paina paina commented Oct 9, 2023

Related to: bufbuild/protovalidate#99

Based on @higebu's idea on the PR, I've implemented isIpPrefix() for C++.

@CLAassistant
Copy link

CLAassistant commented Oct 9, 2023

CLA assistant check
All committers have signed the CLA.

@paina paina mentioned this pull request Oct 9, 2023
4 tasks
rodaine added a commit to bufbuild/protovalidate that referenced this pull request Oct 30, 2023
- [x] protovalidate-go:
bufbuild/protovalidate-go#53
- [x] protovalidate-cc:
bufbuild/protovalidate-cc#45 (via @paina)
- [x] protovalidate-java:
bufbuild/protovalidate-java#39
- [x] protovalidate-python:
bufbuild/protovalidate-python#78

---------

Co-authored-by: Chris Roche <[email protected]>
@rodaine
Copy link
Member

rodaine commented Oct 31, 2023

@paina looks like there's still some wiring missing to define the CEL functions. Want to take a stab it?

@rodaine rodaine requested a review from Alfus October 31, 2023 17:30
@rodaine
Copy link
Member

rodaine commented Oct 31, 2023

Getting an error running the conformance tests locally on MacOS Sonoma:

→ make conformance
GOBIN=/Users/rodaine/src/github.com/bufbuild/protovalidate-cc/.tmp/bin go install \
        github.com/bufbuild/protovalidate/tools/[email protected]
bazel build -c opt //buf/validate/conformance:runner_main && \
.tmp/bin/protovalidate-conformance bazel-bin/buf/validate/conformance/runner_main --strict
INFO: Analyzed target //buf/validate/conformance:runner_main (24 packages loaded, 177 targets configured).
INFO: Found 1 target...
ERROR: /Users/rodaine/src/github.com/bufbuild/protovalidate-cc/buf/validate/internal/BUILD.bazel:103:11: Compiling buf/validate/internal/extra_func.cc failed: (Exit 1): cc_wrapper.sh failed: error executing command (from target //buf/validate/internal:extra_func) external/local_config_cc/cc_wrapper.sh -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -Wunused-but-set-parameter -Wno-free-nonheap-object -fcolor-diagnostics ... (remaining 65 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
buf/validate/internal/extra_func.cc:299:14: error: no member named '__in6_u' in 'in6_addr'
        mask.__in6_u.__u6_addr32[i] = htonl(0xffffffff);
        ~~~~ ^
buf/validate/internal/extra_func.cc:301:14: error: no member named '__in6_u' in 'in6_addr'
        mask.__in6_u.__u6_addr32[i] = htonl(std::pow(2, 32*(i+1) - prefixlen_int) - 1);
        ~~~~ ^
buf/validate/internal/extra_func.cc:303:14: error: no member named '__in6_u' in 'in6_addr'
        mask.__in6_u.__u6_addr32[i] = htonl(0);
        ~~~~ ^
buf/validate/internal/extra_func.cc:307:29: error: no member named '__in6_u' in 'in6_addr'
      if ((sa_six.sin6_addr.__in6_u.__u6_addr32[i] & mask.__in6_u.__u6_addr32[i]) != 0) {
           ~~~~~~~~~~~~~~~~ ^
buf/validate/internal/extra_func.cc:307:59: error: no member named '__in6_u' in 'in6_addr'
      if ((sa_six.sin6_addr.__in6_u.__u6_addr32[i] & mask.__in6_u.__u6_addr32[i]) != 0) {
                                                     ~~~~ ^
5 errors generated.
Target //buf/validate/conformance:runner_main failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 4.636s, Critical Path: 4.26s
INFO: 12 processes: 8 internal, 4 darwin-sandbox.
FAILED: Build did NOT complete successfully
Makefile:46: *** [conformance] error 1

#0  conformance at /Users/rodaine/src/github.com/bufbuild/protovalidate-cc/Makefile:45

Copy link
Member

@jchadwick-buf jchadwick-buf left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I left a couple of comments mostly regarding portability. I also think if possible it would be better to avoid std::pow, which will use floating point math, if using simple bit-twiddling would suffice instead.

buf/validate/internal/BUILD.bazel Outdated Show resolved Hide resolved
buf/validate/internal/extra_func.cc Outdated Show resolved Hide resolved
buf/validate/internal/extra_func.cc Outdated Show resolved Hide resolved
buf/validate/internal/extra_func.cc Outdated Show resolved Hide resolved
buf/validate/internal/extra_func.cc Outdated Show resolved Hide resolved
@Alfus Alfus removed their request for review November 2, 2023 16:04
@rodaine rodaine requested a review from jchadwick-buf November 7, 2023 21:12
Copy link
Member

@jchadwick-buf jchadwick-buf left a comment

Choose a reason for hiding this comment

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

LGTM

@rodaine rodaine merged commit 64da14e into bufbuild:main Nov 7, 2023
3 checks passed
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.

4 participants