ci(3rd-party): drop -Wconversion from omnivoice; widen debug src1_str buffer#23
Merged
Conversation
… buffer Two follow-ups for LLAMA_FATAL_WARNINGS=ON (CI 3rd-party / ubuntu-24-llguidance): omnivoice_lib still tripped `-Werror=conversion` after PR #21: src/llama-adapter.h:86:30: error: conversion from std::unordered_map<...>::size_type (aka unsigned long) to uint32_t may change value [-Werror=conversion] Same rationale as the -Wshadow carve-out — the warning is in upstream llama core headers, not omnivoice code. Drop -Wconversion from omnivoice_lib's compile options. -Wno-sign-conversion is already dropped further down so the variable now has no purpose; remove it. common/debug.cpp tripped -Werror=format-truncation= at line 167: '}' directive output may be truncated writing 1 byte into a region of size between 0 and 127 The src1_str buffer was 256 bytes for a snprintf of '%s{%s}' where src1->name can be up to GGML_MAX_NAME=128 (omnivoice bumps the default 64 to 128 PUBLIC on ggml). GCC can't prove the bound and rejects the write. Widen the buffer to 512 to give the worst-case name + shape string headroom.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to PR #21 —
-DLLAMA_FATAL_WARNINGS=ONexposed two more-Werror=failures onubuntu-24-llguidance:omnivoice_libfailing on-Werror=conversion:The warning is in upstream llama core headers, not omnivoice's code. Drop
-Wconversion(and the now-pointless-Wno-sign-conversion) fromomnivoice_lib, same rationale as the-Wshadowcarve-out from PR fix(omnivoice): drop -Wshadow to unblock LLAMA_FATAL_WARNINGS (CI 3rd-party) #21.common/debug.cpp:167failing on-Werror=format-truncation:src1_strwas 256 bytes;src1->nameis up toGGML_MAX_NAME=128(omnivoice bumps it from 64 PUBLIC on ggml). Widen the buffer to 512 to give the worst-case name + shape string headroom.Test plan