loader should use snprintf() when printing dynamic strings into command_errbuf.
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
like it, except for a few nits.
sys/boot/common/boot.c | ||
---|---|---|
64 ↗ | (On Diff #19455) | Super duper nit alert: No space after sizeof here and elsewhere. |
sys/boot/common/bootstrap.h | ||
40 ↗ | (On Diff #19455) | This isn't needed.... |
sys/boot/common/commands.c | ||
461 ↗ | (On Diff #19455) | { not needed here. optional, so i'll let you keep it if you didn't do this on accident. |
sys/boot/common/bootstrap.h | ||
---|---|---|
40 ↗ | (On Diff #19455) | Actually it is. As the actual variable is defined in command.c, other source files do not see it's size and sizeof() could not be used. Confirmed by compiler - can't argue with it;) |
sys/boot/common/boot.c | ||
---|---|---|
64 ↗ | (On Diff #19455) | From cstyle(9): "Space after keywords (if, while, for, return, switch)." Parentheses there mean an example, not an exact list of keywords to put space after, for exact list you wont need parentheses there at all. sizeof is definitely keyword in C, as are quite many other (I haven't counted them, somewhere was list of 32 keywords;) |
sys/boot/common/commands.c | ||
---|---|---|
461 ↗ | (On Diff #19455) | yes, I know it is optional, the cstyle does allow this for multi-line statements, and in general, it keeps multi-line statements more readable. Altho, perhaps this case is not *that* obvious;) |
sys/boot/common/boot.c | ||
---|---|---|
64 ↗ | (On Diff #19455) | Except sizeof is special and always has been. It's even explicitly documented: "Casts and sizeof's are not followed by a space. " |
sys/boot/common/bootstrap.h | ||
40 ↗ | (On Diff #19455) | Ah, yes. You're right. Forget I said anything. |
sys/boot/common/commands.c | ||
461 ↗ | (On Diff #19455) | As long as it was intentional and not the result of some intermediate step where you needed them, it's cool. You're changing the code in the general area, in some cases you're making things multi-line (though not multi-statement). It's a reasonable change if done on purpose, which you've done. |
sys/boot/common/boot.c | ||
---|---|---|
64 ↗ | (On Diff #19455) | selective reading:D well, you are right there obviously, thanks for pointing it out:) |
I know this was committed, but I wanted to note I was cool with it and too busy to tick the 'approved' box until now.