Path: ...!eternal-september.org!feeder3.eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com>
Newsgroups: comp.lang.c
Subject: Re: Code guidelines
Date: Tue, 3 Sep 2024 14:39:40 -0700
Organization: A noiseless patient Spider
Lines: 125
Message-ID: <vb7vms$3g2ii$1@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>
 <vb76us$3bntp$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 23:39:41 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="8c0285703d08e1fff39effa95f2fa6f6";
	logging-data="3672658"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX19LG+HVsGxFvTLdy8lQd0cSbijObPBqYME="
User-Agent: Mozilla Thunderbird
Cancel-Lock: sha1:ZXs/MThtnuW24zUEfoAxXfoDWOg=
In-Reply-To: <vb76us$3bntp$2@dont-email.me>
Content-Language: en-US
Bytes: 5927

On 9/3/2024 7:37 AM, Thiago Adams wrote:
> 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.

Beware of assert. I had to debug some persons code that said it was 
crashing and no assert was being called. Turned out he had NDEBUG 
defined. Yikes! ;^o