[POSIX] Fix incorrect mirror partial tile reconstruction - #1180
Conversation
| treeP := layout.PartialTileSize(l, idx, treeSize) | ||
| cpP := layout.PartialTileSize(l, idx, cpSize) |
There was a problem hiding this comment.
What if you short-circuit earlier here?
| cpP := layout.PartialTileSize(l, idx, cpSize) | |
| if cpP == 0 { | |
| continue | |
| } | |
| treeP := layout.PartialTileSize(l, idx, treeSize) |
| } else { | ||
| tile := mustReadTile(t, lr, uint64(level), idx, uint8(wantNodes)) | ||
| if len(tile.Nodes) != wantNodes { | ||
| t.Errorf("Expected %d nodes in partial tile %d/%d.%d, got %d", wantNodes, level, idx, wantNodes, len(tile.Nodes)) |
There was a problem hiding this comment.
Should the warning message use uint8(wantNodes) instead of wantNodes in case it's the full tile? Otherwise, it may show .256.
| // ensureGeometry checks that the partial tiles and entry bundles implied by the new size are | ||
| // present in the stored resources. | ||
| // | ||
| // If an implied partial resource is not already present, this function will attempt to create | ||
| // it from a strictly larger resource whose presence is implied by treeSize. |
There was a problem hiding this comment.
Should we add one more line to the code comment to include the new change?
"When cpSize == treeSize the resources are guaranteed present by integration, so there's nothing to do."
There was a problem hiding this comment.
I think that's implied on the method comment, but I've added it as an in-line comment on the condition check.
|
|
||
| return func(yield func(*api.EntryBundle, error) bool) { | ||
| remaining := totalEntries | ||
| dummyEntry := []byte("a") |
There was a problem hiding this comment.
nit: Using []byte("a") doesn't try to catch whether there is any offset. Consider using something like fmt.Appendf(nil, "entry %d", base+i)?
2 participants
This PR addresses an issue with the mirror writer lifecycle implementation in the POSIX driver where it would create an invalid out-of-bounds tile under certain circumstances.
Fixes #1178