Re: Code guidelines

On 03/09/2024 15:33, Thiago Adams wrote:

On 03/09/2024 10:16, David Brown wrote:

On 03/09/2024 14:22, Thiago Adams wrote:

I am of the opinion that if something is clearly specified, you make sure it is true when you are responsible for it - and then you can assume it is true when you use the results.  It makes no sense to me to do something, and then immediately check that it is done.
>
Whenever possible, these specifications should be in code, not comments - using whatever tools and extra features you have available for use. Macros and conditional compilation help make code more portable, and can also let you turn compile-time assumptions into run-time checks during debugging.
>

 Yes, this contract "function don't returns null" is very straightforward for the caller and for the implementer.
 The contract implementation can be checked inside function implementation. (One place to check the contract implementation)
The contract usage, is checked in each caller, but very straightforward.
 On the other hand, struct members have much more places where the contract can be broken. Each function that have a non const access to user->name could break the contract implementation.
The risk is much bigger compared with the return case.

You are asking about "guidelines".  Yes, it is possible for code to break specifications.  The guideline is "don't do that".
If you are mixing untrusted code with trusted code, then you have to check the incoming data just like you do with data from external sources.  But most functions are not at such boundaries.

 So I think it may make sense to have redundant runtime checks, but it must be clear the the "compile time contract" is not that one.
  For instance:
 The first sample my create confusion (is name optional?)
 void f(struct user* user)
{
      if (user->name && strcmp(user->name, "john") == 0)
      {
         //...
      }
  }
 But :
void f(struct user* user)
{
      assert(user->name);
      if (user->name && strcmp(user->name, "john") == 0)
      {
         //...
      }
  }
 would show redundancy but making clear the contract still "name should not be null"
 

No, redundant runtime checks are not helpful.  If they are redundant, they are by definition a waste of effort - either run-time efficiency, or developer efficiency, or both.  And they are a maintenance mess as changes have to be applied in multiple places.
     void f(struct user* user)
     {
         if (!user->name) __builtin_unreachable();
         if (strcmp(user->name, "john") == 0) {
             //...
         }
     }
Now it is /really/ clear that user->name should not be null, and instead of two run-time checks doing the same thing, there are no run-time costs and the compiler may even be able to optimise more from the knowledge that the pointer is not null.
In practice I use a macro called "Assume" here, which will give a hard compile-time error if the compiler can that the assumption is false.  It also supports optional run-time checks depending on a #define'd flag (normally set as a -D compiler flag), as an aid to debugging.
Write things clearly, once, in code.
If the code is boundary code, then you need a check - but then a simple null pointer check is unlikely to be sufficient.  (What if the pointer is not null, but not a valid pointer?  Or it does not point to a string of a suitable length, with suitable character set, etc.?)