A lot of the libsa functions were written back when ANSI C wasn’t a thing. So, I replaced their old, nonstandard parts with their modern-day equivalents.
No functional changes intended.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
ANSIfying definitions has some value, but everything else seems like a complete waste of time to me...
| stand/libsa/ext2fs.c | ||
|---|---|---|
| 802 | This doesn't look very NFC to me | |
| 904 | This is a bit different in nature, whole block ends up being indented... not saying it's better as it is but this seems like busywork for no appreciable gain, the current code is pretty clear | |
| stand/libsa/in_cksum.c | ||
| 91 | & is commutative, this has zero semantic effect | |
| stand/libsa/printf.c | ||
| 75 ↗ | (On Diff #94114) | ??? for all of these |
| stand/libsa/smbios.c | ||
| 215 ↗ | (On Diff #94114) | Yes these variables are redundant. No this is not worthwhile to change. |
| 516 ↗ | (On Diff #94114) | Equivalent, even if the old code goes against our current style(9) |
| stand/libsa/strcasecmp.c | ||
| 55 | Given that tolower is an inline function I doubt this has any effect on the final code generated, these are all simple transformations for half-decent compilers to make. | |
| stand/libsa/strdup.c | ||
| 48–50 | style(9) has all variables declared at the start of the function, with the sole exception being that for loops may declare their counter variable | |
| 49 | You don't need this cast | |
| stand/libsa/in_cksum.c | ||
|---|---|---|
| 91 | just easier to read since & 0xffff is the same as % 0x10000 | |
| stand/libsa/ext2fs.c | ||
|---|---|---|
| 802 | And thus you see one of the problems with posting reviews that are a mess of unrelated, unnecessary changes like this: reviewers find it hard to review them and are more likely to make mistakes. | |
Comment Actions
Can you split this review into at least two. So convert only the K&R style function decls to ANSI and the rest? The rest likely need to be split as well further, but that would at least give us an easy to review chunk. The goto again -> do while likely are a second chunk.
For changes like this where there's relatively little gain from the actual change, having small, committable chunks will speed the process. Extracting the prototypes changes, for example, is more hassle than it's worth for me, but cherry-picking a change isn't so bad.
It's also to MFC this way, and easier to back out should one subset of the changes prove to be bad.