Re: Adding macros to public hdrs, in particular strEQs and strNEs

Read the commit that added them.

I will add more comments when I am no longer on holiday from my laptop.

On 3 Feb 2017 04:36, "Karl Williamson" <public@khwilliamson.com> wrote:

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.

• Adding macros to public hdrs, in particular strEQs and strNEs by Karl Williamson
• Re: Adding macros to public hdrs, in particular strEQs and strNEs by demerphq
• Re: Adding macros to public hdrs, in particular strEQs and strNEs by demerphq