Skip to content

#4574: fix sanity check asyncBatchReadEntries #4579

Merged
StevenLuMT merged 2 commits into
apache:masterfrom
mazzucchi-andrea:LedgerHandle-batchReadEntries-sanity-check
Apr 15, 2025
Merged

#4574: fix sanity check asyncBatchReadEntries #4579
StevenLuMT merged 2 commits into
apache:masterfrom
mazzucchi-andrea:LedgerHandle-batchReadEntries-sanity-check

Conversation

@mazzucchi-andrea

Copy link
Copy Markdown
Contributor

Fix: #4574

Motivation

The little sanity check in the method asyncBatchReadEntries in LedgerHandle does not check if the entrId is equal to or grater
to zero.

Changes

The method validates the input only for startEntry > lastAddConfirmed. This fix makes it compliant to the other reader methods and validates the input like this startEntry < 0 || startEntry > lastAddConfirmed.

@StevenLuMT StevenLuMT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good jobs

@StevenLuMT

StevenLuMT commented Apr 15, 2025

Copy link
Copy Markdown
Member

@mazzucchi-andrea If possible, I think I'd add a startEntry < 0 exception test case

@xiezhx9 xiezhx9 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

IncorrectParameter seems more precise here

@hezhangjian hezhangjian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can startEntry less than zero?

@hezhangjian hezhangjian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mazzucchi-andrea

Copy link
Copy Markdown
Contributor Author

@mazzucchi-andrea If possible, I think I'd add a startEntry < 0 exception test case

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

batchReadEntries raises an unexpected exception

5 participants