neocturne / fastd

Fast and Secure Tunnelling Daemon
Other
115 stars 16 forks source link

Introducing 'const' where args are meant to remain invariant #16

Closed smoe closed 3 years ago

smoe commented 3 years ago

It compiles. And with only very few exceptions this only introduces "const" as a keyword. This is meant to help the the human to understand the API more easily and there is some hope that also the compiler finds it easier to inline the code more efficiently. And I just like it - more for a C++ app than for pure C - but I like it everywhere.

Hoping to match your taste. Steffen

neocturne commented 3 years ago

const in that position doesn't have any effect for function prototypes (as these arguments are copied); it only affects the inside of the function. It can even differ between the prototype and function definition: The following code is completely valid and compiles without warnings:

int foo(int foo);

int foo(int const foo) {
        return foo;
}

As such, the const attribute is not part of the functions' API - specifying it doesn't make the API any clearer. To the contrary: the fact that the argument variable is not changed inside the function is an implementation detail that should not be visible to the API user, as this is breaking abstraction. It can also not have any effect on optimizations (for optimizations, const becomes relevant when it describes data behind pointers).

For function definitions (in contrast to prototypes), the attribute may have some merit, but only to catch programming errors; all local variables that aren't mutated could be declared as const by the same logic - function arguments aren't special in this regard.

Ultimately it is a matter of taste. For C code, declaring local variables as const is very unusual, and I try to keep the coding style close to what other projects do. For this reason, I have to reject this PR, sorry.