This is an archive of the discontinued Mercurial Phabricator instance.

hg-core: use `u32` instead of `i32` in `Chunk` (D8958#inline-15001 followup)
ClosedPublic

Authored by acezar on Sep 28 2020, 9:48 AM.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

acezar created this revision.Sep 28 2020, 9:48 AM
Alphare added inline comments.
rust/hg-core/src/revlog/patch.rs
25–28

Is the offset always positive? Same question for all affected methods.

acezar marked an inline comment as done.Sep 29 2020, 4:32 AM
acezar added inline comments.
rust/hg-core/src/revlog/patch.rs
25–28

The start of the replaced chunk cannot be negative. Negative start would mean that the replacement start before the start of the data. Same for the other methods.

Alphare added inline comments.Sep 29 2020, 4:35 AM
rust/hg-core/src/revlog/patch.rs
25–28

I'm only asking because if self.start is 0, and offset is -1, the as u32 conversion would put the result to u32::max_value(). But maybe that never happens?

acezar marked an inline comment as done.Sep 29 2020, 4:40 AM
acezar added inline comments.
rust/hg-core/src/revlog/patch.rs
25–28

Theoretically no. But if there is a mistake... maybe checked_add should be used

acezar updated this revision to Diff 22912.Sep 29 2020, 4:57 AM
martinvonz added inline comments.
rust/hg-core/src/revlog/patch.rs
114

nit: delta seems like a clearer name

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.