search

stm32duino / FatFs

FatFs is a generic FAT file system module for small embedded systems. The FatFs is written in compliance with ANSI C and completely separated from the disk I/O layer. Therefore it is independent of hardware architecture.
BSD 3-Clause "New" or "Revised" License
91 stars 30 forks source link

Unitialized variable in f_read() #6

Closed vipako closed 3 years ago

vipako commented 3 years ago

In f_read() function is variable declaration line:

UINT rcnt, cc, csect; ... // few lines of code ... for ( ; btr; / Repeat until all data read / rbuff += rcnt, fp->fptr += rcnt, *br += rcnt, btr -= rcnt) {

problem is that variable rcnt is not initialized and is assigned to random value.

Correct version: UINT rcnt = 0, cc, csect;

fpistm commented 3 years ago

It is initialized to 0 by the compiler.

vipako commented 3 years ago

Could you please send me information which C compiler is doing it. Because I personally doesn't know any compiler which initialize automatic(local) variables automatically to zero, because C specification is saying: The value of an uninitialized local variable in C is indeterminate and reading it can invoke undefined behavior. Please fix the bug.

fpistm commented 3 years ago

Well I don't remember exactly why and how. Maybe related to the fact it is a typedef storage calss but not the subject here. I'm agreed with you that it is better to init local variable anyway, this part of code come from original FatFs (based on release R0.12C from http://elm-chan.org/fsw/ff/arc/ff12c.zip), this library only package it to Arduino library format. The same "issue" is present on the original code and is not an issue. I don't know what lead you to open this specific issue anyway I will not modify this part of code it should be raised to original source code. And it seems not be an issue as I guess if it is a problem this would be solved since a long time.

Moreover it will be overwrite during the next update and keeping patch up to date is a nightmare. 😉

fpistm commented 3 years ago

any update on this ? Else I will close it.