develooper Front page | perl.perl5.porters | Postings from December 2020

Re: Why is perl_RnW1_mutex_t.readers_count unsigned?

Thread Previous | Thread Next
From:
Karl Williamson
Date:
December 31, 2020 16:13
Subject:
Re: Why is perl_RnW1_mutex_t.readers_count unsigned?
Message ID:
7fffca49-911c-8684-03a8-4b443fb3b23e@khwilliamson.com
On 12/30/20 2:55 PM, Craig A. Berry wrote:
> We have a relatively new struct in perl.h that looks like this:
> 
> typedef struct {
>    perl_mutex lock;
>    perl_cond wakeup;
>    Size_t readers_count;
> } perl_RnW1_mutex_t;
> 
> Meanwhile, over in thread.h, we have a number of macros that do things
> like the following to make sure the unsigned readers_count does not go
> negative:
> 
>              if ((mutex)->readers_count <= 0) {                      \
>                  assert((mutex)->readers_count == 0);                \
>                  (mutex)->readers_count = 0;                         \
>                  break;                                              \
>              }                                                       \
> 
> That seems pretty pointless to me since an unsigned value can't go
> negative. Should we make readers_count SSize_t rather than Size_t, or
> should we get rid of the check for negative values?
> 
> I ask because the compiler I'm using complains as follows in a couple
> dozen places.
> 
>      GETENV_UNLOCK;
> ...^
> %CC-I-QUESTCOMPARE, In this statement, the unsigned expression
> "(&PL_env_mutex)->readers_count" is being compared with a relational
> operator to a constant whose value is not greater than zero.  This
> might not be what you intended.
> at line number 2802 in file SMOKE_BUILD_ROOT:[blead]inline.h;1
> 
> This recently broke the build until I relaxed Time::HiRes's
> configuration code and still causes at least one test failure
> (porting/extrefs.t).  If the intent was to limit the number of readers
> on a mutex, there must be some more reasonable way to specify the
> range than depending on integer wraparound to let us know there are
> too many. I think we would run into resource exhaustion long before
> getting anywhere near INT_MAX readers.
> 

Like most things, it was not thinking it through.  Changed by 
41b10d901d383944620dc9fe58a82531022cc937

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About