Page MenuHomeFreeBSD

Remove compiler hacks
Needs ReviewPublic

Authored by gfunni234_gmail.com on Aug 24 2021, 6:03 PM.

Details

Reviewers
imp
jrtc27
dim
Group Reviewers
transport
Summary

Some code clearly was written to please the compiler and avoid a warning, as Werror is enabled, or trick older compilers into producing better code generation. However, the way this was achieved came at the cost of readability.

These hacks aren’t needed anymore and I have proposed much better solutions.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Yet again so much noise I'm not even bothering to give individual comments for

sys/libkern/fnmatch.c
65

This is harder to read. Previously it was if (early_bail_out_exception) return (0); do_normal_case. Now everything's been mashed into one if.

97

Who cares whether people use else or not when the if has a return? This is just a waste of time.

155

???

sys/netgraph/ng_socket.c
431

So after your aversion to goto in another revision you're happy to introduce one here, and one that jumps *into* a block at that? There is nothing wrong with the existing code, it's how I would write it.

sys/libkern/fnmatch.c
138–141

That's removing the common idiom *p++ for traversing a string. Harder to read.

155

Both terms are equivalent, because the result is thrown away.
In principle p++ is more complex, because it has to return the old value, but in this case the common idiom is the postfix operator.

162

const char **newp is different for the caller. It requires the caller to provide a const char. Are you sure, that all calling instances do so?

sys/netgraph/ng_socket.c
431

If you want to make the compiler happy, initialize the len variable during declaration. Then you have to opportunity to get rid of this case at all.

int len = 0;
...
if (sap != NULL) {
    if (sap->sg_len > NG_NODESIZE ...

Everything else can be kept.

432

If you use goto to skip code, then please remove the else branch, too.

445

Because a preinitialized len is 0 in the case of an error, your suggested simplification will work.

sys/netinet/tcp_lro.c
1452–1469

x and y have always identical values, so they can be used interchangeably.
Why not remove y completely? It would be much clearer to read.

usr.sbin/bsnmpd/tools/libbsnmptools/bsnmpmap.c
514

Use initialization during declaration to make the compiler happy.
This feature was introduced in the 90th by C compilers.