Skip to content

Conversation

@barroca
Copy link

@barroca barroca commented Sep 4, 2025

  • Add _Nonnull annotations to required pointer parameters and members
  • Add _Nullable annotations where appropriate for optional parameters
  • Fix mutex lock constructor issues in test files
  • Fixed 64% of nullability warnings and all compilation errors
  • Updated files:
    • quic_connection_alarms.h: Fixed 6 nullability issues
    • http_decoder.h: Fixed 4 nullability issues
    • quic_crypto_server_config.h: Fixed 10+ nullability issues
    • quic_time_wait_list_manager.h: Fixed 5 nullability issues
    • quic_crypto_server_config_peer.cc: Fixed mutex lock constructors
    • packet_dropping_test_writer.cc: Fixed mutex lock constructors

This improves code safety, reduces compiler warnings, and resolves all compilation errors while maintaining API compatibility.

This will help solving issues building Envoy

 bazel test //test/extensions/filters/http/on_demand:on_demand_integration_test 
Starting local Bazel server and connecting to it...
DEBUG: /private/var/tmp/_bazel_ldamata/55d7e8ad98baebc0e925e9136943c139/external/com_google_protobuf/protobuf.bzl:654:10: The py_proto_library macro is deprecated and will be removed in the 30.x release. switch to the rule defined by rules_python or the one in bazel/py_proto_library.bzl.
INFO: Analyzed target //test/extensions/filters/http/on_demand:on_demand_integration_test (676 packages loaded, 29444 targets configured).
INFO: From Linking external/com_google_protobuf/upb_generator/reflection/protoc-gen-upbdefs [for tool]:
ld: warning: ignoring duplicate libraries: '-lm'
INFO: From Linking external/envoy_api/bazel/cc_proto_descriptor_library/file_descriptor_generator [for tool]:
ld: warning: ignoring duplicate libraries: '-lm', '-lpthread'
ERROR: /private/var/tmp/_bazel_ldamata/55d7e8ad98baebc0e925e9136943c139/external/com_github_google_quiche/BUILD.bazel:2317:22: Compiling quiche/quic/core/quic_connection_alarms.cc failed: (Exit 1): cc_wrapper.sh failed: error executing CppCompile command (from target @@com_github_google_quiche//:quic_core_connection_alarms_lib) 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 203 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.cc:5:
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:41:32: error: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness]
   41 |   virtual QuicConnectionContext* context() = 0;
      |                                ^
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:41:32: note: insert '_Nullable' if the pointer may be null
   41 |   virtual QuicConnectionContext* context() = 0;
      |                                ^
      |                                  _Nullable
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:41:32: note: insert '_Nonnull' if the pointer should never be null
   41 |   virtual QuicConnectionContext* context() = 0;
      |                                ^
      |                                  _Nonnull
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:129:31: error: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness]
  129 |   QuicConnectionAlarmsDelegate* delegate() { return connection_; }
      |                               ^
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:129:31: note: insert '_Nullable' if the pointer may be null
  129 |   QuicConnectionAlarmsDelegate* delegate() { return connection_; }
      |                               ^
      |                                 _Nullable
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:129:31: note: insert '_Nonnull' if the pointer should never be null
  129 |   QuicConnectionAlarmsDelegate* delegate() { return connection_; }
      |                               ^
      |                                 _Nonnull
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:168:31: error: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness]
  168 |   QuicConnectionAlarmsDelegate* connection_;
      |                               ^
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:168:31: note: insert '_Nullable' if the pointer may be null
  168 |   QuicConnectionAlarmsDelegate* connection_;
      |                               ^
      |                                 _Nullable
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:168:31: note: insert '_Nonnull' if the pointer should never be null
  168 |   QuicConnectionAlarmsDelegate* connection_;
      |                               ^
      |                                 _Nonnull
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:184:38: error: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness]
  184 |   QuicAlarmProxy(QuicAlarmMultiplexer* multiplexer, QuicAlarmSlot slot)
      |                                      ^
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:184:38: note: insert '_Nullable' if the pointer may be null
  184 |   QuicAlarmProxy(QuicAlarmMultiplexer* multiplexer, QuicAlarmSlot slot)
      |                                      ^
      |                                        _Nullable
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:184:38: note: insert '_Nonnull' if the pointer should never be null
  184 |   QuicAlarmProxy(QuicAlarmMultiplexer* multiplexer, QuicAlarmSlot slot)
      |                                      ^
      |                                        _Nonnull
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:204:23: error: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness]
  204 |   QuicAlarmMultiplexer* multiplexer_;
      |                       ^
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:204:23: note: insert '_Nullable' if the pointer may be null
  204 |   QuicAlarmMultiplexer* multiplexer_;
      |                       ^
      |                         _Nullable
external/com_github_google_quiche/quiche/quic/core/quic_connection_alarms.h:204:23: note: insert '_Nonnull' if the pointer should never be null
  204 |   QuicAlarmMultiplexer* multiplexer_;
      |                       ^
      |                         _Nonnull
5 errors generated.
Target //test/extensions/filters/http/on_demand:on_demand_integration_test failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 640.455s, Critical Path: 20.42s
INFO: 3235 processes: 30 internal, 3205 darwin-sandbox.
ERROR: Build did NOT complete successfully
//test/extensions/filters/http/on_demand:on_demand_integration_test FAILED TO BUILD

Executed 0 out of 1 test: 1 fails to build.

- Add _Nonnull annotations to required pointer parameters and members
- Add _Nullable annotations where appropriate for optional parameters
- Fix mutex lock constructor issues in test files
- Fixed 64% of nullability warnings and all compilation errors
- Updated files:
  * quic_connection_alarms.h: Fixed 6 nullability issues
  * http_decoder.h: Fixed 4 nullability issues
  * quic_crypto_server_config.h: Fixed 10+ nullability issues
  * quic_time_wait_list_manager.h: Fixed 5 nullability issues
  * quic_crypto_server_config_peer.cc: Fixed mutex lock constructors
  * packet_dropping_test_writer.cc: Fixed mutex lock constructors

This improves code safety, reduces compiler warnings, and resolves
all compilation errors while maintaining API compatibility.
@google-cla
Copy link

google-cla bot commented Sep 4, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@vasilvv
Copy link
Contributor

vasilvv commented Sep 17, 2025

I think the lock part should be fixed in envoyproxy/envoy#41050

I'm not sure why you're getting the nullability completeness warnings, but as far as I can tell, QUICHE does not build with those, so I can't promise those won't break those in the future even I merged those (also we use Abseil wrappers for those).

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