Can we remove the expandSparse call in WriteAt?

[schomatis]

Now that we are including it in Sync (#33).

It would also be helpful to review the entire function itself.

[kjzz]

go-unixfs/mod/dagmodifier.go

Lines 92 to 101 in 7731bf7

	} else if uint64(offset) != dm.curWrOff {
	size, err := dm.Size()
	if err != nil {
	return 0, err
	}
	if offset > size {
	err := dm.expandSparse(offset - size)
	if err != nil {
	return 0, err
	}

In the WriteAt , it use DagModifier.Size() to get size.But the DagModifier.Size() may be larger than fileSize(ipld.node).May it will casue somthing wrong?

[schomatis]

In the WriteAt , it use DagModifier.Size() to get size.

Yes, it returns the size taking into account the write buffer that's not flushed yet.

But the DagModifier.Size() may be larger than fileSize(ipld.node).May it will casue somthing wrong?

I don't think so, there are two different checks to call expandSparse with the corresponding different size methods (that should be clearly distinguished) but it seems to me that the check in Sync should cover the check in WriteAt, that is, if we're writing past the potential size of the file (taking into account the write buffer) then definitely the (now adjusted writeStart) should trigger the expandSparse in Sync.

[schomatis]

@kjzz Could you remove that expandSparse call and check if it triggers any errors in the tests? (That is not enough to be sure it can be safely removed from the code but it's a good first step.)

[kjzz]

@schomatis It can passed all test if we remove the expandSparse call in writeAt.

[Jorropo]

This repository is no longer maintained and has been copied over to Boxo. In an effort to avoid noise and crippling in the Boxo repo from the weight of issues of the past, we are closing most issues and PRs in this repo. Please feel free to open a new issue in Boxo (and reference this issue) if resolving this issue is still critical for unblocking or improving your usecase.

You can learn more in the FAQs for the Boxo repo copying/consolidation effort.

---

See ipfs/boxo#224 too