Check for shadowing in lfs.c and ensure clean compilation#1150
Check for shadowing in lfs.c and ensure clean compilation#1150DarukaThirumurugan wants to merge 1 commit intolittlefs-project:masterfrom
Conversation
BenBE
left a comment
BenBE
left a comment
There was a problem hiding this comment.
The naming of the introduced variables doesn't really improve clarity per se.
A better approach would be checking which of these cases may actually allow to skip these additional variables and use the outer scoped one (safely). That way this would even improve on stack frame size.
Nothing against this PR (I'm all for enforcing -Wshadow, it's just that this currently doesn't look clean either … So more refinement might be good.
There was a problem hiding this comment.
Unrelated new test, that doesn't actually test anything in LittleFS.
| err1 = lfs_tortoise_detectcycles(pdir, &tortoise); | ||
| if (err1 < 0) { |
There was a problem hiding this comment.
Inlining this check would eliminate the need for err1 entirely AFAICS.
| err1 = lfs_tortoise_detectcycles(pdir, &tortoise); | |
| if (err1 < 0) { | |
| if (lfs_tortoise_detectcycles(pdir, &tortoise) < 0) { |
| #include "lfs_util.h" | ||
|
|
||
|
|
||
|
|
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, | ||
| f->ctz.head, f->ctz.size, cb, data); | ||
| if (err) { | ||
| if (err_traverse ) { |
There was a problem hiding this comment.
Introduces white space/formatting change.
| if (err_traverse ) { | |
| if (err_traverse) { |
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, | ||
| f->ctz.head, f->ctz.size, cb, data); | ||
| if (err) { | ||
| if (err_traverse ) { |
There was a problem hiding this comment.
Introduces white space/formatting change.
| if (err_traverse ) { | |
| if (err_traverse) { |
|
|
||
| if ((f->flags & LFS_F_WRITING) && !(f->flags & LFS_F_INLINE)) { | ||
| int err = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, | ||
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, |
There was a problem hiding this comment.
Introduces white space/formatting change.
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, | |
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, |
|
|
||
| if ((f->flags & LFS_F_WRITING) && !(f->flags & LFS_F_INLINE)) { | ||
| int err = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, | ||
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, |
There was a problem hiding this comment.
Introduces white space/formatting change.
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, | |
| int err_traverse = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache, |
| if (err_traverse ) { | ||
| return err_traverse ; |
There was a problem hiding this comment.
Introduces white space/formatting change.
| if (err_traverse ) { | |
| return err_traverse ; | |
| if (err_traverse) { | |
| return err_traverse; |
|
Hi @DarukaThirumurugan, I appreciate the pull request, but IMO this is an unproductive warning and does more harm than good. See #873 and #873 (comment). |
3 participants
Refactor: cleared variable shadowing in lfs.c
Identified and resolved instances of variable shadowing in lfs.c.
This improves code readability and reduces the risk of bugs
due to accidental re-use of variable names in inner scopes.
Used -Wshadow flag to detect issues and verified successful
compilation with no warnings.
In the original code there are many shadows occured as a warning
After Altering
There is no shadow occurs