switch to non-zero PTHREAD_*_INITIALIZER

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

switch to non-zero PTHREAD_*_INITIALIZER

Poul-Henning Kamp
Right now most of our PTHREAD_*_INITIALIZER macros are defined as NULL.

This is a bad choice from a code quality point of view, because it means
that

        pthread_t my_mutex;

and

        pthread_t my_mutes = PTHREAD_MUTEX_INITIALIZER;

act the same, which they are not.

I suggest that we should change the macros to a non-NULL value, and
add a check for NULL values which emit a warning about the lack of
initialization.

Comments ?

--
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
[hidden email]         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: switch to non-zero PTHREAD_*_INITIALIZER

Konstantin Belousov
On Mon, Feb 11, 2019 at 09:43:42AM +0000, Poul-Henning Kamp wrote:

> Right now most of our PTHREAD_*_INITIALIZER macros are defined as NULL.
>
> This is a bad choice from a code quality point of view, because it means
> that
>
> pthread_t my_mutex;
>
> and
>
> pthread_t my_mutes = PTHREAD_MUTEX_INITIALIZER;
>
> act the same, which they are not.
>
> I suggest that we should change the macros to a non-NULL value, and
> add a check for NULL values which emit a warning about the lack of
> initialization.
>
> Comments ?

This would make the startup (or more) of current binaries too noisy and
perhaps even break the applications that depend on specific output from
the subordinate processes.

I wanted to reorganize static initializizers for some time, esp. to add
specific initializers like RECURSIVE_NP and similar. This requires more
changes in libthr, particular to the shared and destroyed mutexes canary
values, which is quite delicate thing to do.  So far I did not spend time
on this.
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: switch to non-zero PTHREAD_*_INITIALIZER

Poul-Henning Kamp
--------
In message <[hidden email]>, Konstantin Belousov writes:

>On Mon, Feb 11, 2019 at 09:43:42AM +0000, Poul-Henning Kamp wrote:
>> Right now most of our PTHREAD_*_INITIALIZER macros are defined as NULL.
>>
>> This is a bad choice from a code quality point of view, because it means
>> that
>>
>> pthread_t my_mutex;
>>
>> and
>>
>> pthread_t my_mutes = PTHREAD_MUTEX_INITIALIZER;
>>
>> act the same, which they are not.
>>
>> I suggest that we should change the macros to a non-NULL value, and
>> add a check for NULL values which emit a warning about the lack of
>> initialization.
>>
>> Comments ?
>
>This would make the startup (or more) of current binaries too noisy and
>perhaps even break the applications that depend on specific output from
>the subordinate processes.

Right, we probably should make it tweakable with something like /etc/malloc.conf.


--
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
[hidden email]         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: switch to non-zero PTHREAD_*_INITIALIZER

Daniel Eischen
In reply to this post by Poul-Henning Kamp
On Mon, 11 Feb 2019, Poul-Henning Kamp wrote:

> Right now most of our PTHREAD_*_INITIALIZER macros are defined as NULL.
>
> This is a bad choice from a code quality point of view, because it means
> that
>
> pthread_t my_mutex;
>
> and
>
> pthread_t my_mutes = PTHREAD_MUTEX_INITIALIZER;
>
> act the same, which they are not.
>
> I suggest that we should change the macros to a non-NULL value, and
> add a check for NULL values which emit a warning about the lack of
> initialization.
>
> Comments ?

You'll get warnings from anything that hasn't be rebuilt with the
new initializers, including other languages that have overlays
to the same C structures (well, really ours are pointers to
structs).

I'm not really against this, but I'd rather see our mutex, condvar,
etc, be real structs instead of pointers to structs.  This would
hopefully get rid of the whole malloc mess inside libthr and simplify
pthread_XXX_setpshared().  I'd think that we've evolved the pthread
API enough to pick some large enough storage size for our structs
such that we won't have to change it again.  Of course, even with
versioned libraries, this change would probably require a library
version bump.

--
DE
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"