Skip to content

fix: disallow negative chunk_size parameter in comdb2_files#5992

Open
SChakravorti21 wants to merge 1 commit into
bloomberg:mainfrom
SChakravorti21:comdb2-files-vtable
Open

fix: disallow negative chunk_size parameter in comdb2_files#5992
SChakravorti21 wants to merge 1 commit into
bloomberg:mainfrom
SChakravorti21:comdb2-files-vtable

Conversation

@SChakravorti21
Copy link
Copy Markdown
Contributor

Summary

  • Validate that chunk_size is positive in files_util_filter() before passing it to the file reading pipeline
  • A negative value (e.g. -1) would propagate through set_chunk_size/dbfile_set_chunk_size and trigger the assertion f->chunk_size > 0 in ar_wrap.c:read_write_file(), aborting the server process
  • Added test test_comdb2_files_negative_chunk_size that verifies the server stays alive after a chunk_size = -1 query

Test plan

  • make comdb2_files passes locally (all existing tests + new negative chunk_size test)
  • CI passes

🤖 Generated with Claude Code

Validate that `chunk_size` is positive before passing it to the file reading
pipeline. A negative value (e.g. `-1`) would propagate through
`set_chunk_size`/`dbfile_set_chunk_size` and trigger an assertion in
`ar_wrap.c:read_write_file()`, aborting the server process.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Shoumyo Chakravorti <schakravorti@bloomberg.net>
Copy link
Copy Markdown

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Cbuild submission: Success ✓.
Regression testing: 4/675 tests failed ⚠.

The first 10 failing tests are:
sp_snapshot_generated
consumer_non_atomic_default_consumer_generated **quarantined**
remotecreate_twopc_generated
remotecreate
truncatesc_offline_generated [timeout] **quarantined**
reco-ddlk-sql [timeout] **quarantined**

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