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

> 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.

On 3 February 2017 at 03:05, demerphq <demerphq@gmail.com> wrote:
> Read the commit that added them.

I see I was confused. I thought I had squashed

777fa2cb699b74cad6714d203572533dbd583976

together with

62946e08d24766c73755a22909c985cbf0a1b970


> 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.

It is good to bring it up, but I/we add macros all the time, and so
far I don't recall any bugs from collisions. The rationale seemed
sound to me, and I was following various already established
conventions, or as far as I could figure out what the convention was.

> 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.

Fair enough.

> 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.

This is fair too. Although I did sort of explain them in the comment.
I admit I completely forgot to add docs for these functions,

> 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.)

Yes correct.

> 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.

BAH. I get it now. Look two lines up.

strnEQ()

I forgot/missed the @!#$! 'n'.

If you consider

strEQs()

should have been

strnEQs()

then I think convention is preserved.

Also the docs for strnEQ() and strnNE() are wrong, they behave just as
the new macros I added do, they check that first 'l' characters are
equal or not.

> Putting proposals on p5p may invite bikeshedding, but not putting them
> there invites worse.

I'm sorry this rankled you, and obviously my omission of the 'n'
proves your point.

But I think that there is a long tradition of this, and the waters
have been muddied by many people, including both of us. ;-)

Consider that in 2007 Nicholas Clark added

memEQs(s1, l, s2)

Then in September 2016 you added memLT(s1,s2,l) and friends. (Notice
the argument order difference?)

Then in October 2016 I added _memEQs() and strEQs and friends.


> 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.

Indeed. There are many places where these tricks are very useful. As
can be seen in the collection of macros we have.

> 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.

Huh? I dont follow you? Do you mean

strnNE() and strnEQ()

Those have been in perl since Larry Wall days.

IMO the mistake was forgetting the 'n'. (Is that what you meant when
you said 'M'?)

Naming it strnEQs() imo follows established convention, albeit
somewhat cryptically.

> 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.

Now I am confused. I thought you were complaining about strEQs()?
Didnt you want to rename it?

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

strNE() has been around for ages, since

^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  478)
#define strNE(s1,s2) (strcmp(s1,s
^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  479)
#define strEQ(s1,s2) (!strcmp(s1,
^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  480)
#define strLT(s1,s2) (strcmp(s1,s
^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  481)
#define strLE(s1,s2) (strcmp(s1,s
^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  482)
#define strGT(s1,s2) (strcmp(s1,s
^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  483)
#define strGE(s1,s2) (strcmp(s1,s
62946e08 (Yves Orton               2016-10-19 10:30:44 +0200  484)
^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  485)
#define strnNE(s1,s2,l) (strncmp(
^8d063cd (Larry Wall               1987-12-18 00:00:00 +0000  486)
#define strnEQ(s1,s2,l) (!strncmp

IMO if we are going to follow convention establed for strnNE() we
should rename strNEs to strnNEs().

I think it would be fine to add more descriptive equivalents that
perhaps have slightly different apis, but i am not sure that I was so
wrong (besides omitting the 'n') in how I implemented things.

On the other hand I am not opposed to any cleanups you want to
propose, I guess it depends on how you view the precedents.

Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

• 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