On Wed, May 09, 2007 at 01:06:49PM -0700, chromatic wrote:
> On Wednesday 09 May 2007 12:53:57 Nicholas Clark wrote:
> 
> > On Tue, May 01, 2007 at 04:41:22PM -0700, [EMAIL PROTECTED] wrote:
> > > +
> > > +#define STRING_IS_NULL(s) ((s) == NULL)
> > > +#define STRING_IS_EMPTY(s) !(int)(s)->strlen

> Does !(int)(s)->strlen really scan as quickly and easily as STRING_IS_EMPTY?

Mmm, yes, thinking about it more...
What's that int cast doing there?

Smells like a bug. Either (s)->strlen is going to be zero, in which case
! of it is true, or it's going to be in the range INT_MIN to INT_MAX, in
which case it's not true, or it's going to be outside that range, in which
case the cast is undefined behaviour. (because it's signed)

I've not checked, and I'm not sure if it's going to be easy to do so, but
I assume that the cast was moved into the macro as part of refactoring,
and has been in the code for some time.

So, !s->strlen does scan as quickly and easily.

s == NULL is also more tersely written as !s, which, I feel, is also clearer
to regular C programmers.

I've also intentionally left the parentheses off, as once you aren't using
macros, you can choose not to use them on simple expressions.

> > Arguably one of the mistakes of Perl 5 was to use too many macros, which
> > unintentionally contributes to obfuscating the code.
> 
> It's not as if *these* are SvPVNL and SvPVZ, or was that SVpvNL or SvPv 
> or....?

Yes. Those ones. But after about 5 years I started to see the patterns in them.

Clearly 5 years isn't a rapid learning curve.

Nicholas Clark

Reply via email to