dynamic_modules: use size_t for length of buffers
#37357
Conversation
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
@phlax I found in the test log that somewhere in the CI container we have a very old clang toolchain (10.0.0). can we identify and remove it? If we can put the /opt/llvm/bin/clang into |
Signed-off-by: Takeshi Yoneda <[email protected]>
mathetake
changed the title
size_t for length of buffers
| // This is Envoy CI specific: Check if "/opt/llvm/bin/clang" exists, and if it does, set the | ||
| // CLANG_PATH environment variable. CLANG_PATH is for clang-sys used by bindgen: | ||
| // https://github.com/KyleMayes/clang-sys?tab=readme-ov-file#environment-variables | ||
| // | ||
| // "/opt/llvm/bin/clang" exists in Envoy CI containers. If the clang doesn't exist there, bindgen | ||
| // will try to use the system clang from PATH. So, this doesn't affect the local builds. | ||
| // In any case, clang must be found to build the bindings. | ||
| // | ||
| // TODO: add /opt/llvm/bin to PATH in the CI containers. That would be a better solution. | ||
| if std::fs::metadata("/opt/llvm/bin/clang").is_ok() { | ||
| env::set_var("CLANG_PATH", "/opt/llvm/bin/clang"); | ||
| } |
There was a problem hiding this comment.
Note: this is the only functional addition and others are formatting. This is needed otherwise bindgen will end up using the very old (i think some installation garbage in the past) clang and cannot find the stdlibs on RBE envs.
|
/retest |
Signed-off-by: Takeshi Yoneda <[email protected]>
im reasonably confident that is not the case with CI - it was until relatively recently with one of the wasm engines, but that has gone - ndk (for mobile/android) also brings its own toolchain but iirc not that old could it be this? $ git grep llvm-10
.bazelci/presubmit.yml:# - "bazel/setup_clang.sh /usr/lib/llvm-10" |
|
actually probably not - its commented out |
|
re hardcoding |
|
yeah sorry nvm on the PATH stuff |
Commit Message: dynamic_modules: use size_t for length of buffers
Additional Description:
Previously,
intwas used for the length of buffer/string, which wasobviously a bug as its size is 16-bit and signed.
size_tis usuallyused for expressing the memory buffer by a pair of pointer and the length,
so this commit switches to use it.
Risk Level: n/a
Testing: done
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a