File::Glob GLOB_NOCHECK documentation unclear

[p5pRT]

Migrated from rt.perl.org#126239 (status was 'resolved')

Searchable as RT126239$

[p5pRT]

From @epa

Created by @epa

The documentation for File​::Glob says

  "GLOB_NOCHECK"   If the pattern does not match any pathname\, then bsd_glob()   returns a list consisting of only the pattern. If   "GLOB_QUOTE" is set\, its effect is present in the pattern   returned.

I understood this to mean​: if you set the GLOB_NOCHECK flag\, and the pattern does not match any pathname\, then bsd_glob() returns a list of only the pattern. By implication\, this means that if GLOB_NOCHECK is not set\, and the pattern does not match\, then bsd_glob() returns the empty list.

However\, the behaviour looks like the opposite​:

% perl -MFile​::Glob -E '@​g = File​::Glob​::bsd_glob("nonexistent"\, GLOB_NOCHECK); say foreach @​g' % perl -MFile​::Glob -E '@​g = File​::Glob​::bsd_glob("nonexistent"); say foreach @​g' nonexistent

If GLOB_NOCHECK is set then you get an empty result from a non-matching pattern. The default\, if that flag is not set\, is to pass the pattern back if it fails to match.

This isn't consistent with the documentation - and it appears inconsistent with POSIX glob(3)\, which returns the original pattern if GLOB_NOCHECK is set and GLOB_NOMATCH otherwise.

I don't know whether the best way forward is to fix the documentation to match what File​::Glob does (and note the inconsistency with POSIX) or to fix the behaviour to match the docs (and POSIX).

Perl Info

``` Flags: category=library severity=low module=File::Glob Site configuration information for perl 5.20.3: Configured by Red Hat, Inc. at Mon Sep 14 07:37:15 UTC 2015. Summary of my perl5 (revision 5 version 20 subversion 3) configuration: Platform: osname=linux, osvers=4.1.6-100.fc21.x86_64, archname=x86_64-linux-thread-multi uname='linux buildhw-02.phx2.fedoraproject.org 4.1.6-100.fc21.x86_64 #1 smp mon aug 17 22:20:37 utc 2015 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Doptimize=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -Dlddlflags=-shared -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wl,-z,relro -Dshrpdir=/usr/lib64 -DDEBUGGING=-g -Dversion=5.20.3 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic', cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='5.1.1 20150618 (Red Hat 5.1.1-4)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='gcc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib64 /lib64 /usr/lib64 /usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib libs=-lpthread -lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -lresolv -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.21.so, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.21' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags' cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wl,-z,relro -L/usr/local/lib' Locally applied patches: Fedora Patch1: Removes date check, Fedora/RHEL specific Fedora Patch3: support for libdir64 Fedora Patch4: use libresolv instead of libbind Fedora Patch5: USE_MM_LD_RUN_PATH Fedora Patch6: Skip hostname tests, due to builders not being network capable Fedora Patch7: Dont run one io test due to random builder failures Fedora Patch15: Define SONAME for libperl.so Fedora Patch16: Install libperl.so to -Dshrpdir value Fedora Patch22: Document Math::BigInt::CalcEmu requires Math::BigInt (CPAN RT#85015) Fedora Patch25: Use stronger algorithm needed for FIPS in t/op/crypt.t (RT#121591) Fedora Patch26: Make *DBM_File desctructors thread-safe (RT#61912) Fedora Patch27: Report inaccesible file on failed require (RT#123270) Fedora Patch28: Use stronger algorithm needed for FIPS in t/op/taint.t (RT#123338) Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux @INC for perl 5.20.3: /home/eda/share/perl5 /home/eda/lib/perl5/ /home/eda/lib64/perl5/ /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 . Environment for perl 5.20.3: HOME=/home/eda LANG=en_GB.UTF-8 LANGUAGE (unset) LC_COLLATE=C LC_CTYPE=en_GB.UTF-8 LC_MESSAGES=en_GB.UTF-8 LC_MONETARY=en_GB.UTF-8 LC_NUMERIC=en_GB.UTF-8 LC_TIME=en_GB.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/eda/bin:/home/eda/bin:/usr/local/bin:/usr/bin:/sbin:/usr/sbin:/sbin:/usr/sbin PERL5LIB=/home/eda/share/perl5:/home/eda/lib/perl5/:/home/eda/lib64/perl5/ PERL_BADLANG (unset) SHELL=/bin/bash -- Ed Avis Please ignore autogenerated disclaimer below this point. This email is intended only for the person to whom it is addressed and may contain confidential information. Any retransmission, copying, disclosure or other use of, this information by persons other than the intended recipient is prohibited. If you received this email in error, please contact the sender and delete the material. This email is for information only and is not intended as an offer or solicitation for the purchase or sale of any financial instrument. Wadhwani Asset Management LLP is a Limited Liability Partnership registered in England (OC303168) with registered office at 40 Berkeley Square, 3rd Floor, London, W1J 5AL. It is authorised and regulated by the Financial Conduct Authority. ```

[p5pRT]

From @iabyn

On Thu\, Oct 01\, 2015 at 09​:22​:44AM -0700\, Ed Avis wrote​:

# New Ticket Created by "Ed Avis" # Please include the string​: [perl #126239] # in the subject line of all future correspondence about this issue. # \<URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=126239 >

This is a bug report for perl from eda@​waniasset.com\, generated with the help of perlbug 1.40 running under perl 5.20.3.

----------------------------------------------------------------- [Please describe your issue here]

The documentation for File​::Glob says

   "GLOB\_NOCHECK"
       If the pattern does not match any pathname\, then bsd\_glob\(\)
       returns a list consisting of only the pattern\.  If
       "GLOB\_QUOTE" is set\, its effect is present in the pattern
       returned\.

I understood this to mean​: if you set the GLOB_NOCHECK flag\, and the pattern does not match any pathname\, then bsd_glob() returns a list of only the pattern. By implication\, this means that if GLOB_NOCHECK is not set\, and the pattern does not match\, then bsd_glob() returns the empty list.

However\, the behaviour looks like the opposite​:

% perl -MFile​::Glob -E '@​g = File​::Glob​::bsd_glob("nonexistent"\, GLOB_NOCHECK); say foreach @​g' % perl -MFile​::Glob -E '@​g = File​::Glob​::bsd_glob("nonexistent"); say foreach @​g' nonexistent

It's actually doing the right thing; it's a combination of two things that's causing your example to go astray. First\, GLOB_NOCHECK isn't exported by default\, so you're actually using the value '0'; second\, in the absence of a second arg\, the flags of bsd_glob() doesn't default to zero\, but rather to​:

  If the second argument is omitted\, C\<GLOB_CSH> (or C\<GLOB_CSH|GLOB_NOCASE>   on VMS and DOSish systems) is used by default.

which happens to include GLOB_NOMAGIC\, which is nearly equivalent to GLOB_NOCHECK.

This code​:

use File​::Glob qw(bsd_glob GLOB_NOCHECK GLOB_NOMAGIC); my @​g; for my $f ("nonexistent"\, "nonexstent*") {   @​g = bsd_glob($f); print "bsd_glob($f)​: [@​g]\n";   @​g = bsd_glob($f\, 0); print "bsd_glob($f\,0)​: [@​g]\n";   @​g = bsd_glob($f\, GLOB_NOCHECK); print "bsd_glob($f\,GLOB_NOCHECK)​: [@​g]\n";   @​g = bsd_glob($f\, GLOB_NOMAGIC); print "bsd_glob($f\,GLOB_NOMAGIC)​: [@​g]\n"; }

produces this output​:

  bsd_glob(nonexistent)​: [nonexistent]   bsd_glob(nonexistent\,0)​: []   bsd_glob(nonexistent\,GLOB_NOCHECK)​: [nonexistent]   bsd_glob(nonexistent\,GLOB_NOMAGIC)​: [nonexistent]   bsd_glob(nonexstent*)​: []   bsd_glob(nonexstent*\,0)​: []   bsd_glob(nonexstent*\,GLOB_NOCHECK)​: [nonexstent*]   bsd_glob(nonexstent*\,GLOB_NOMAGIC)​: []

which I think is the correct behaviour.

-- More than any other time in history\, mankind faces a crossroads. One path leads to despair and utter hopelessness. The other\, to total extinction. Let us pray we have the wisdom to choose correctly.   -- Woody Allen

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @epa

Thanks for spotting the problems with my example code.

First\, GLOB_NOCHECK isn't exported by default\, so you're actually using the value '0';

That's not quite true. As a bareword\, it becomes the string value "GLOB_NOCHECK". But then this string gets treated as equivalent to zero. This is unfortunate given that the File​::Glob documentation seems to imply that strings can be passed​: on my Linux system\, 'man File​::Glob' displays

  POSIX FLAGS   The POSIX defined flags for bsd_glob() are​:

  "GLOB_ERR"   Force bsd_glob() to return an error...

Those "" quotation marks seem to suggest that strings are passed to bsd_glob(). I know that the quotation marks have only appeared in there as an artefact of the perldoc-to-manpage conversion. And I know that the flags argument should be an integer not a string. But it's all a bit confusing as documented.

Given this series of unfortunate events\, could I suggest that if bsd_glob() is given a string instead of an integer for its flags argument\, it should warn? For extra niceness it could handle strings such as "GLOB_NOCHECK" as being equivalent to the corresponding flag\, but still with a warning that you should change your code. In my opinion\, this warning from File​::Glob should appear even if $^W is false.

Once you start doing bitwise operations on barewords things get even stranger​:

% perl -E '$x = GLOB_NOCHECK | GLOB_BRACE; say $x' GLOB_N_CKECK

I don't suggest File​::Glob should attempt to handle that; it should just warn that the unknown string will be treated as zero.

Thanks for pointing out the default set of flags. I had overlooked that\, assuming that they were all off by default. Should I send a documentation patch to make this clearer?

[p5pRT]

From @iabyn

On Thu\, Nov 19\, 2015 at 09​:08​:37AM -0800\, Ed Avis via RT wrote​:

Thanks for spotting the problems with my example code.

First\, GLOB_NOCHECK isn't exported by default\, so you're actually using the value '0';

That's not quite true. As a bareword\, it becomes the string value "GLOB_NOCHECK". But then this string gets treated as equivalent to zero. This is unfortunate given that the File​::Glob documentation seems to imply that strings can be passed​: on my Linux system\, 'man File​::Glob' displays

POSIX FLAGS The POSIX defined flags for bsd_glob() are​:

   "GLOB\_ERR"
       Force bsd\_glob\(\) to return an error\.\.\.

Those "" quotation marks seem to suggest that strings are passed to bsd_glob(). I know that the quotation marks have only appeared in there as an artefact of the perldoc-to-manpage conversion. And I know that the flags argument should be an integer not a string. But it's all a bit confusing as documented.

Given this series of unfortunate events\, could I suggest that if bsd_glob() is given a string instead of an integer for its flags argument\, it should warn?

But it already does​:

  $ perl -MFile​::Glob -wE 'File​::Glob​::bsd_glob("foo"\, GLOB_NOCHECK);'   Argument "GLOB_NOCHECK" isn't numeric in subroutine entry at -e line 1.   $

For extra niceness it could handle strings such as "GLOB_NOCHECK" as being equivalent to the corresponding flag\, but still with a warning that you should change your code. In my opinion\, this warning from File​::Glob should appear even if $^W is false.

Once you start doing bitwise operations on barewords things get even stranger​:

% perl -E '$x = GLOB_NOCHECK | GLOB_BRACE; say $x' GLOB_N_CKECK

I don't suggest File​::Glob should attempt to handle that; it should just warn that the unknown string will be treated as zero.

It already does.

So I don't think anything needs changing.

Thanks for pointing out the default set of flags. I had overlooked that\, assuming that they were all off by default. Should I send a documentation patch to make this clearer?

Yes\, that would be fine.

-- "Strange women lying in ponds distributing swords is no basis for a system of government. Supreme executive power derives from a mandate from the masses\, not from some farcical aquatic ceremony."   -- Dennis\, "Monty Python and the Holy Grail"

[p5pRT]

From @epa

I have taken the liberty of putting explicit parens on the constants in example code\, which would prevent any naively copied code (in oneliners\, etc) from hitting the problem I encountered. What do you think?

Inline Patch

```diff diff --git a/ext/File-Glob/Glob.pm b/ext/File-Glob/Glob.pm index c23b7df..da5596f 100644 --- a/ext/File-Glob/Glob.pm +++ b/ext/File-Glob/Glob.pm @@ -91,7 +91,7 @@ File::Glob - Perl extension for BSD glob routine use File::Glob ':bsd_glob'; @list = bsd_glob('*.[ch]'); - $homedir = bsd_glob('~gnat', GLOB_TILDE | GLOB_ERR); + $homedir = bsd_glob('~gnat', GLOB_TILDE() | GLOB_ERR()); if (GLOB_ERROR) { # an error occurred reading $homedir @@ -176,10 +176,14 @@ means this will loop forever: =head3 C This function, which is included in the two export tags listed above, -takes one or two arguments. The first is the glob pattern. The second is -a set of flags ORed together. The available flags are listed below under -L. If the second argument is omitted, C (or -C on VMS and DOSish systems) is used by default. +takes one or two arguments. The first is the glob pattern. The +second, if given, is a set of flags ORed together. The available +flags and the default set of flags are listed below under L. + +Remember that to use the named constants for flags you must import +them, for example with C<:bsd_glob> described above. If not imported, +and Perl's warnings are not turned on, then the constants will be +treated as bareword strings, which won't do what you what. =head3 C<:nocase> and C<:case> @@ -196,7 +200,9 @@ uses this internally. =head2 POSIX FLAGS -The POSIX defined flags for bsd_glob() are: +If no flags argument is give then C is set, and on VMS and +Windows systems, C too. Otherwise the flags to use are +determined solely by the flags argument. The POSIX defined flags are: =over 4 @@ -268,7 +274,7 @@ Expand patterns that start with '~' to user name home directories. =item C For convenience, C is a synonym for -C. +C. =back ```

[p5pRT]

From @ap

* Dave Mitchell \davem@&#8203;iabyn\.com [2015-11-23 12​:30]​:

On Thu\, Nov 19\, 2015 at 09​:08​:37AM -0800\, Ed Avis via RT wrote​:

Given this series of unfortunate events\, could I suggest that if bsd_glob() is given a string instead of an integer for its flags argument\, it should warn?

But it already does​:

$ perl \-MFile​::Glob \-wE 'File​::Glob​::bsd\_glob\("foo"\, GLOB\_NOCHECK\);'
Argument "GLOB\_NOCHECK" isn't numeric in subroutine entry at \-e line 1\.
$

So I don't think anything needs changing.

But that using the global -w flag\, which is generally not used any more.

Absent of it\, as in most code nowadays\, File​::Glob would have to enable warnings for itself in order for it to actually warn about this issue.

So most users will never see this warning.

I think File​::Glob does need changing.

Thanks for pointing out the default set of flags. I had overlooked that\, assuming that they were all off by default. Should I send a documentation patch to make this clearer?

Yes\, that would be fine.

If the warning actually got raised then that would tip users off to the problem without the docs having to get bloated.

Regards\, -- Aristotle Pagaltzis // \<http​://plasmasturm.org/>

[p5pRT]

From @ap

* Ed Avis via RT \perlbug\-followup@&#8203;perl\.org [2015-11-23 12​:50]​:

I have taken the liberty of putting explicit parens on the constants in example code\, which would prevent any naively copied code (in oneliners\, etc) from hitting the problem I encountered. What do you think?

I think we should teach people to turn on strict instead of making all the code examples uglier.

[p5pRT]

From @epa

I think we should teach people to turn on strict instead of making all the code examples uglier.

I 'use strict' in all my programs except for oneliners with perl -E. I agree that in general it is to be encouraged.

But you are also right that if File​::Glob printed a warning (and ideally one more beginner-friendly than "isn't numeric") then it wouldn't be necessary to add any of these defensive measures or gotcha warnings to the documentation.

[p5pRT]

From @iabyn

On Mon\, Nov 23\, 2015 at 02​:52​:14PM +0100\, Aristotle Pagaltzis wrote​:

* Dave Mitchell \davem@&#8203;iabyn\.com [2015-11-23 12​:30]​:

On Thu\, Nov 19\, 2015 at 09​:08​:37AM -0800\, Ed Avis via RT wrote​:

Given this series of unfortunate events\, could I suggest that if bsd_glob() is given a string instead of an integer for its flags argument\, it should warn?

But it already does​:

$ perl \-MFile​::Glob \-wE 'File​::Glob​::bsd\_glob\("foo"\, GLOB\_NOCHECK\);'
Argument "GLOB\_NOCHECK" isn't numeric in subroutine entry at \-e line 1\.
$

So I don't think anything needs changing.

But that using the global -w flag\, which is generally not used any more.

Absent of it\, as in most code nowadays\, File​::Glob would have to enable warnings for itself in order for it to actually warn about this issue.

So most users will never see this warning.

The warning is checked in the caller's scope\, since it's an XS sub​:

$ perl -MFile​::Glob -e'use warnings; File​::Glob​::bsd_glob("nonexistent"\, GLOB_NOCHECK)' Argument "GLOB_NOCHECK" isn't numeric in subroutine entry at -e line 1. $

-- My Dad used to say 'always fight fire with fire'\, which is probably why he got thrown out of the fire brigade.

[p5pRT]

From @ap

* Dave Mitchell \davem@&#8203;iabyn\.com [2015-11-23 15​:55]​:

The warning is checked in the caller's scope\, since it's an XS sub​:

Oh!

So this will only catch out people who turn on *neither* strictures *nor* warnings.

Let’s say I’m not inclined to consider File​::Glob the problem there.

[p5pRT]

From @epa

The only time I have run File​::Glob with neither strictures or warnings is when writing oneliners to reproduce this 'bug'... so it is all a bit self-referential.

Still\, surely you agree that a warning message like

  The flags argument to bsd_glob has been passed as the string 'GLOB_NOCASE' but it should be a numeric value. Perhaps you need to import the named constants\, or consider running with 'use strict'?

would be an improvement on the current "isn't numeric" warning? If I write a patch to make the warning message more idiot-proof\, will you consider applying it?

[p5pRT]

From @ap

* Ed Avis via RT \perlbug\-followup@&#8203;perl\.org [2015-11-23 16​:20]​:

Surely you agree that a warning message like

The flags argument to bsd\_glob has been passed as the string
'GLOB\_NOCASE' but it should be a numeric value\.  Perhaps you need
to import the named constants\, or consider running with 'use
strict'?

would be an improvement on the current "isn't numeric" warning?

I do. (Well aside from the fact that that is too long for a warning.) I’m still not sure it’s really necessary but I’m not opposed. It should be terser\, though\, with the full explanation (if one is needed) under DIAGNOSTICS in the POD. (So you get your wish for a doc patch in the end! :-))

If I write a patch to make the warning message more idiot-proof\, will you consider applying it?

I wouldn’t feel qualified to apply an XS patch\, myself\, so Dave or co will have to answer that.

[p5pRT]

From @epa

My plan was to rename the current XS routine to _bsd_glob_impl or something like that\, and have bsd_glob be a small Perl wrapper to it that sanity checks the arguments. Unless someone more familiar with XS and/or File​::Glob thinks that is a bad idea.

[p5pRT]

From @iabyn

On Mon\, Nov 23\, 2015 at 07​:46​:37AM -0800\, Ed Avis via RT wrote​:

My plan was to rename the current XS routine to _bsd_glob_impl or something like that\, and have bsd_glob be a small Perl wrapper to it that sanity checks the arguments. Unless someone more familiar with XS and/or File​::Glob thinks that is a bad idea.

I personally really don't see the point in making bsd_glob check whether its args are strings or not. You already get a warning\, and this is no different than any of the other places which use symbolic constants\, e.g.

  flock $fh\, LOCK_SH;

Should we be fixing up all them too?

-- Art is anything that has a label (especially if the label is "untitled 1")

[p5pRT]

From @epa

There is a small difference in that flock() will fail with 'invalid argument' if passed a duff file locking mode; bsd_glob() treats the string as a valid set of flags (all zero) and continues.

[p5pRT]

From @demerphq

On 23 November 2015 at 18​:54\, Ed Avis via RT \perlbug\-followup@&#8203;perl\.org wrote​:

There is a small difference in that flock() will fail with 'invalid argument' if passed a duff file locking mode; bsd_glob() treats the string as a valid set of flags (all zero) and continues.

IIRC we have had other cases where this was also true and we fixed them.

Yves

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

[p5pRT]

From @iabyn

On Mon\, Nov 23\, 2015 at 07​:20​:44PM +0100\, demerphq wrote​:

On 23 November 2015 at 18​:54\, Ed Avis via RT \perlbug\-followup@&#8203;perl\.org wrote​:

There is a small difference in that flock() will fail with 'invalid argument' if passed a duff file locking mode; bsd_glob() treats the string as a valid set of flags (all zero) and continues.

IIRC we have had other cases where this was also true and we fixed them.

The difference between flock() and bsd_glob() is that for the former\, zero isn't a valid value for the flags arg\, while it is for the latter.

Another example of a function using symbolic constants with zero as a valid flag is sysopen​:

  $ perl -we'sysopen my $fh\, "/dev/null"\, O_RDONLY; print "ok\n"'   Argument "O_RDONLY" isn't numeric in sysopen at -e line 1.   ok   $

-- A power surge on the Bridge is rapidly and correctly diagnosed as a faulty capacitor by the highly-trained and competent engineering staff.   -- Things That Never Happen in "Star Trek" #9

[p5pRT]

From @epa

FWIW\, I would favour making sysopen warn more explicitly about 'the flags argument was passed as a string; import the constants; and please use strict'. And print that warning whether or not the general numification warning is enabled with -w etc. With this kind of stupid programming error you really can't be too explicit or too helpful in the diagnostics.

But I take the general point​: if sysopen and friends don't have a separate check for flags passed as strings\, it may be too much to demand that bsd_glob should have one. If I were maintaining the module on CPAN I would add the warning anyway; better to have it than not to have it IMHO. But as it is I must defer to the maintainer's opinion.

[p5pRT]

From @epa

I think the decision in this bug was not to change the code\, but could the documentation patch in \https://rt-archive.perl.org/perl5/Ticket/Attachment/1376745/739203/ be applied please?

[p5pRT]

From @iabyn

On Mon\, Apr 04\, 2016 at 07​:49​:12AM -0700\, Ed Avis via RT wrote​:

I think the decision in this bug was not to change the code\, but could the documentation patch in \https://rt-archive.perl.org/perl5/Ticket/Attachment/1376745/739203/ be applied please?

I think we decided that replacing all the constants FOO in the docs with FOO() wasn't necessary\, as only in the absence of *both* use strict and use warnings would the code compile and run cleanly when using an unimported constant.

-- That he said that that that that is is is debatable\, is debatable.

[p5pRT]

From @epa

Dave Mitchell \<davem \ iabyn.com> writes​:

I think we decided that replacing all the constants FOO in the docs with FOO() wasn't necessary\, as only in the absence of *both* use strict and use warnings would the code compile and run cleanly when using an unimported constant.

OK\, could you apply the rest of the patch please?

(FWIW\, the default behaviour for oneliners is indeed to run without both strict and warnings. Really\, either that behaviour should change\, or code examples should be written taking the oneliner default environment into account. My preference would be for warnings\, but not strict\, to be enabled by default in oneliners.)

-- Ed Avis \eda@&#8203;waniasset\.com

[p5pRT]

From @iabyn

On Tue\, Apr 05\, 2016 at 11​:42​:03AM +0000\, Ed Avis wrote​:

Dave Mitchell \<davem \ iabyn.com> writes​:

I think we decided that replacing all the constants FOO in the docs with FOO() wasn't necessary\, as only in the absence of *both* use strict and use warnings would the code compile and run cleanly when using an unimported constant.

OK\, could you apply the rest of the patch please?

Could you provide an updated patch?

(FWIW\, the default behaviour for oneliners is indeed to run without both strict and warnings. Really\, either that behaviour should change\, or code examples should be written taking the oneliner default environment into account. My preference would be for warnings\, but not strict\, to be enabled by default in oneliners.)

My preference would be to leave -e as-is.

-- You never really learn to swear until you learn to drive.

[p5pRT]

From @epa

--- a/ext/File-Glob/Glob.pm +++ b/ext/File-Glob/Glob.pm @​@​ -176\,10 +176\,14 @​@​ means this will loop forever​: =head3 C\<bsd_glob>

This function\, which is included in the two export tags listed above\, -takes one or two arguments. The first is the glob pattern. The second is -a set of flags ORed together. The available flags are listed below under -L\</POSIX FLAGS>. If the second argument is omitted\, C\<GLOB_CSH> (or -C\<GLOB_CSH|GLOB_NOCASE> on VMS and DOSish systems) is used by default. +takes one or two arguments. The first is the glob pattern. The +second\, if given\, is a set of flags ORed together. The available +flags and the default set of flags are listed below under L\</POSIX FLAGS>. + +Remember that to use the named constants for flags you must import +them\, for example with C\<​:bsd_glob> described above. If not imported\, +and Perl's warnings are not turned on\, then the constants will be +treated as bareword strings\, which won't do what you what.

=head3 C\<​:nocase> and C\<​:case>

@​@​ -196\,7 +200\,9 @​@​ uses this internally.

=head2 POSIX FLAGS

-The POSIX defined flags for bsd_glob() are​: +If no flags argument is give then C\<GLOB_CSH> is set\, and on VMS and +Windows systems\, C\<GLOB_NOCASE> too. Otherwise the flags to use are +determined solely by the flags argument. The POSIX defined flags are​:

=over 4

[p5pRT]

From @ap

* Ed Avis via RT \perlbug\-followup@&#8203;perl\.org [2016-04-05 15​:27]​:

+Remember that to use the named constants for flags you must import +them\, for example with C\<​:bsd_glob> described above.

Do File​::Glob’s docs really have to explain that you can’t use something you haven’t imported…?

                                                  If not imported\,

+and Perl's warnings are not turned on\, then the constants will be +treated as bareword strings\, which won't do what you what.

You mean if the subs stricture is not turned on? Whether or not you have warnings turned on doesn’t make any difference to how bareword strings are treated.

Regards\, -- Aristotle Pagaltzis // \<http​://plasmasturm.org/>

[p5pRT]

From @epa

Thanks Aristotle P.\, here's a corrected patch​:

--- a/ext/File-Glob/Glob.pm +++ b/ext/File-Glob/Glob.pm \ \ -176\,10 +176\,14 \ \ means this will loop forever​: =head3 C\<bsd_glob>

This function\, which is included in the two export tags listed above\, -takes one or two arguments. The first is the glob pattern. The second is -a set of flags ORed together. The available flags are listed below under -L\</POSIX FLAGS>. If the second argument is omitted\, C\<GLOB_CSH> (or -C\<GLOB_CSH|GLOB_NOCASE> on VMS and DOSish systems) is used by default. +takes one or two arguments. The first is the glob pattern. The +second\, if given\, is a set of flags ORed together. The available +flags and the default set of flags are listed below under L\</POSIX FLAGS>. + +Remember that to use the named constants for flags you must import +them\, for example with C\<​:bsd_glob> described above. If not imported\, +and C\ is not in effect\, then the constants will be +treated as bareword strings\, which won't do what you what.

=head3 C\<​:nocase> and C\<​:case>

\ \ -196\,7 +200\,9 \ \ uses this internally.

=head2 POSIX FLAGS

-The POSIX defined flags for bsd_glob() are​: +If no flags argument is give then C\<GLOB_CSH> is set\, and on VMS and +Windows systems\, C\<GLOB_NOCASE> too. Otherwise the flags to use are +determined solely by the flags argument. The POSIX defined flags are​:

=over 4

[p5pRT]

From @iabyn

On Tue\, Apr 05\, 2016 at 08​:24​:50PM +0000\, Ed Avis wrote​:

Thanks Aristotle P.\, here's a corrected patch​:

Thanks\, applied as v5.25.1-185-g8fe4bbc

-- Art is anything that has a label (especially if the label is "untitled 1")

[p5pRT]

@iabyn - Status changed from 'open' to 'resolved'