Path: eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: Thiago Adams <thiago.adams@gmail.com>
Newsgroups: comp.lang.c
Subject: Re: Code guidelines
Date: Tue, 3 Sep 2024 11:37:16 -0300
Organization: A noiseless patient Spider
Lines: 127
Message-ID: <vb76us$3bntp$2@dont-email.me>
References: <vb6v1t$3b5mb$1@dont-email.me> <vb726n$3b4rq$1@dont-email.me>
 <vb736j$3b5mb$2@dont-email.me> <vb75c5$3bu07$1@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:37:17 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="15b67f073a3e4783414ee664431ab3cb";
	logging-data="3530681"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX18sQJLYlFC1N9OHCX3Qj6gc20ptIKrE0RM="
User-Agent: Mozilla Thunderbird
Cancel-Lock: sha1:BbYV4MylQCR9TQkBEQ17Rts2vqQ=
Content-Language: en-US
In-Reply-To: <vb75c5$3bu07$1@dont-email.me>

On 03/09/2024 11:10, David Brown wrote:
> 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".
>

The guidelines are more about the usage of runtime checks, are they 
required? optional? etc.. when to use etc...


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

yes ..agree.

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

But we know , when the code is new (under development) the contract may 
not be so clear or change very frequently.


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

The macro I need is something like

"I know this check is redundant, but I will add it anyway" "this check 
is redundant but according with the contract"

assert/assume is more

"trust me compiler, this is what happens" someone else is checking this 
we don't need to check again.