-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
shell/vfs: Handle print failure in genfile cmd gracefully #20595
Conversation
17057e6
to
92b7856
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This means we just silently ignore the truncation if it happens?
sys/shell/cmds/vfs.c
Outdated
@@ -744,7 +744,19 @@ static void _write_block(int fd, unsigned bs, unsigned i) | |||
char block[bs]; | |||
char *buf = block; | |||
|
|||
buf += snprintf(buf, bs, "|%03u|", i); | |||
int size_wanted = snprintf(buf, bs, "|%03u|", i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the return value of this is 5 %03u
is constant width unless it failed -> bs needs to be 6 to have blocks big enough ( fit this print and a /n )
this would be more readable (to me) if instead of pointer arithmetics, adresses of array positions would be used (in stead of *buf have unsigned pos)
*buf + size
vs
&blocks[pos]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm not entirely sure but while thinking of it, I found a neat little optimization. I moved the memset into the if-clause, please have a look.
Yes, I am not sure what would be best, so I thought: Not writing to a fs is better than writing truncated garbage to the fs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handles snprintf
return value better than before avoiding mishaps
bb54c04
to
8bd1de8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expected results:
(any, 3, 8) will write "|0\n"
(any, 5, 8) will write "|008\n"
(any, 6, 8) will write "|008|\n"
(any, 9, 8) will write "|008|\0\0\0\n"
(any, 3, 8000) will write "|8\n"
(any, 5, 8000) will write "|800\n"
(any, 6, 8000) will write "|8000\n"
(any, 9, 8000) will write "|8000|\0\0\n"
Contribution description
Hey 🐡
snprintf
turned out to be difficult to get right.Testing procedure
Try
make -C tests/sys/vfs_default/ all-asan term
and play with `genfileIssues/PRs references