Bug in truncate function

[agudallago]

Hello,

I was trying to truncate a file using the function spiffs_object_truncate() and I noticed it was not working properly with smaller files. (I tried to truncate a 51 bytes file to 5, for example).

So I noticed this line inside its implementation:

if (cur_size - SPIFFS_DATA_PAGE_SIZE(fs) >= new_size)

In this case, both cur_size and new_size are u32_t. For this reason, even though the left argument in the comparison should be negative, it is not, it is a a very big unsigned int and therefore this condition becomes true when it should be false.

So I changed this line to the following and the issue seemed to be fixed.

if ((s32_t)cur_size - (s32_t)(SPIFFS_DATA_PAGE_SIZE(fs)) >= (s32_t)new_size)

Please confirm that this spotted issue is correct!

Thanks

[yihcdaso-yeskela]

I think, we can stay unsigned

if (cur_size >= new_size + SPIFFS_DATA_PAGE_SIZE(fs))

Not tested!!!!

[pellepl]

Hmm yes, you're right. My bad, an attempt to get rid of warnings, a bit to
hasty it seems. I'll try it out. Thanks!
Den 23 sep 2015 16:18 skrev "Aleksey Osadchiy" notifications@github.com:

Think, we can stay unsigned

if (cur_size >= new_size + SPIFFS_DATA_PAGE_SIZE(fs))

Not tested!!!!

—
Reply to this email directly or view it on GitHub
#48 (comment).

[pellepl]

Well, that was a bit trickier than I thought. Some tests fail after modification - I need to dig a bit deeper in this. Again, thanks for finding this!
I guess it has never been a problem as the API only exports remove (i.e. truncate to zero). I'll roll up the sleeves and get dirty.

[agudallago]

As I said. I think that my fix works pretty good. Am I mistaken?
On 23 Sep 2015 6:57 pm, "Peter Andersson" notifications@github.com wrote:

Well, that was a bit trickier than I thought. Some tests fail after
modification - I need to dig a bit deeper in this. Again, thanks for
finding this!
I guess it has never been a problem as the API only exports remove (i.e.
truncate to zero). I'll roll up the sleeves and get dirty.

—
Reply to this email directly or view it on GitHub
#48 (comment).

[pellepl]

After I applied the fix (any of them) the regression tests fail - it seems something messes up the LUTs. Which is weird, because the fix should be ok. That's why I have to investigate further.

[pellepl]

Update: it seems the fix works when truncating but messes up when fully removing files. Digging on.

[pellepl]

Ok, fixed. Needed some more massaging than just casting, more idiocies from my side.

[pellepl]

Oh and thanks yet again ;)

[agudallago]

It's been a pleasure