Path: ...!2.eu.feeder.erje.net!feeder.erje.net!eternal-september.org!feeder3.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 10:33:06 -0300
Organization: A noiseless patient Spider
Lines: 148
Message-ID: <vb736j$3b5mb$2@dont-email.me>
References: <vb6v1t$3b5mb$1@dont-email.me> <vb726n$3b4rq$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 15:33:07 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="15b67f073a3e4783414ee664431ab3cb";
	logging-data="3512011"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX1+1Q/rCU8rm2Z+9cJ8/b16bjEIsa9rCm8k="
User-Agent: Mozilla Thunderbird
Cancel-Lock: sha1:lNQA0DySRYSfBFUvbqMIfvqStAQ=
Content-Language: en-US
In-Reply-To: <vb726n$3b4rq$1@dont-email.me>
Bytes: 4569

On 03/09/2024 10:16, David Brown wrote:
> On 03/09/2024 14:22, Thiago Adams wrote:
>> Witch style (F1, F2 ..) better represents the guidelines of your code?
>>
>>
>> //The function f never returns nullptr
>> int * f();
>>
>> void F1() {
>>     int* p = f();
>>
>>     if (p == nullptr) {
>>        assert(false);
>>        return;
>>     }
>>     // using *p ...
>> }
>>
>> void F2()
>> {
>>     int* p = f();
>>     assert(p != nullptr);
>>     // using *p ...
>> }
>>
>> void F3()
>> {
>>     int* p = f();
>>     // using *p ...
>> }
>>
>> void F4()
>> {
>>     int* p = f();
>>     if (p) {
>>      // using *p ...
>>     }
>> }
>>
> 
> My preferred style would be :
> 
> int * f() __attribute__((returns_nonnull));
> 
> Then F3 is appropriate.
> 
> (I would also have "(void)" in the declaration, unless I was using C23.)
> 



>>
>>
>> The same question but for struct members
>>
>>
>>
>> struct user {
>>
>>    //Name is required, cannot be null.
>>    char * name;
>> };
>>
>> void F1(struct user* user) {
>>
>>     if (user->name == nullptr) {
>>        assert(false);
>>        return;
>>     }
>>     // using user->name ...
>> }
>>
>> void F2(struct user* user) {
>>     assert(user->name != nullptr);
>>     // using user->name ...
>> }
>>
>> void F3(struct user* user) {
>>     // using user->name ...
>> }
>>
>> void F4(struct user* user) {
>>     if (user->name) {
>>       // using user->name ...
>>     }
>> }
>>
> 
> void F5(struct user * user) {
>      if (!user->name) __builtin_unreachable();
>      // using user->name;
> }
> 
> 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.

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"