Path: ...!weretis.net!feeder8.news.weretis.net!eternal-september.org!feeder3.eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: David Brown <david.brown@hesbynett.no>
Newsgroups: comp.lang.c
Subject: Re: Code guidelines
Date: Tue, 3 Sep 2024 16:10:13 +0200
Organization: A noiseless patient Spider
Lines: 98
Message-ID: <vb75c5$3bu07$1@dont-email.me>
References: <vb6v1t$3b5mb$1@dont-email.me> <vb726n$3b4rq$1@dont-email.me>
 <vb736j$3b5mb$2@dont-email.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Injection-Date: Tue, 03 Sep 2024 16:10:14 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="93d7d6b8e3b40fe75b60c4526d162769";
	logging-data="3536903"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX18XVWQQV1XbknKK7IFF8FYo/qECviCpeAg="
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Cancel-Lock: sha1:d0tZZqcS6XZGIA3WMTJg9SHb9oU=
In-Reply-To: <vb736j$3b5mb$2@dont-email.me>
Content-Language: en-GB
Bytes: 4826

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.?)