develooper Front page | perl.perl5.porters | Postings from February 2017

Adding macros to public hdrs, in particular strEQs and strNEs

Thread Next
From:
Karl Williamson
Date:
February 2, 2017 20:36
Subject:
Adding macros to public hdrs, in particular strEQs and strNEs
Message ID:
57830da1-0dbf-e12b-4835-08ca35490fda@khwilliamson.com
Commit 62946e08d24766c73755a22909c985cbf0a1b970 added strEQs and strNEs 
to handy.h without any comment as to what their usage is.

There are several problems with this.

One is that adding unguarded macros to hdr files immediately places them 
in the API.  I believe that most if not all API changes/additions should 
first be vetted on this list.  Maybe that should be made into a policy. 
I certainly haven't always followed this rule this in the past.

Second, API changes generally should be treated as experimental, and 
soaked before going prime time.  Guarding new macros by things like

  #if defined(PERL_IN_REGCOMP_C) || defined(PERL_IN_UTF8_C)

should suffice, and are better than using PERL_CORE, as that is more 
likely to have XS cheaters.  If things work out, we can remove the guards.

Third, failing to document the intended use is a bad idea.  I've done it 
myself when I thought the usage was obvious from the name, and then 
found others' code that mis-used it, apparently because I was blinded by 
my familiarity with the issue.  Another example is cBOOL, which was 
added without comments.  It took me a while to grok it, and I added 
comments when I did.  Since, I have seen on irc and in emails to this 
list that other people have relied on my comments to decipher things.

Consider strEQs and strNEs in particular.  They were added at nearly the 
same time as memEQs and memNEs (the latter two did have comments to 
suggest their use).  One would expect from the parallelness of the names 
that the two sets would have parallel intent.

  memEQs(s1, l, s2)

compares for equality the string s1 (of length 'l' and possibly 
containing embedded NUL characters) against the literal string s2 (the 
's' in the macro name is meant to imply literalness.)

I would have expected the str ones to do the same, but without embedded 
NULs.

But I was wrong. The reality is that strEQs returns TRUE iff s1 is of 
the form

  s2 . foo

for all foo.  If you're saying, "Of course, that's obvious", I don't 
believe you.  Putting proposals on p5p may invite bikeshedding, but not 
putting them there invites worse.

The initial commits using these macros show that they serve a useful 
purpose.  If you have string literals, the programmer shouldn't have to 
manually count the number of characters in them.  Looking to see if a 
path begins with /dev or a package variable begins with main:: are 
typical uses of strncmp that these macros replace.

So here's my proposal.

Rename strEQs to be something like strBEGs (better alternative names 
solicited).  Get rid of strNEs, as it is uncommon, and N is ambiguous 
and is used in some existing functions to mean "not", and in others to 
mean "Nul" or "Not-Nul".  This was a mistake that should not be extended 
into new names.

Create new strEQs and strNEs which act similarly to memEQs and memNEs, 
like this:

/* strEQs(s1, s1_len, s2)
  * Check for equality between NUL-terminated string 's1' against the string
  * constant "s2".  strlen(s1) is known to be 's1_len'.  Returns 1 if 
equal; 0
  * if not. */

There are about a dozen places in core which could use this macro, 
leading to cleaner code.

I also think that strNE should be changed to return 1 or 0; instead of 
non-zero or 0.

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