Skip to content

Commit 6d28b2f

Browse files
maukemauke
committed
make chdir() return true/false as its documentation claims
Previously it would return integer 1 and 0, not booleans. Fixes Perl#22365.
1 parent 7ea9cde commit 6d28b2f

File tree

7 files changed

+46
-35
lines changed

7 files changed

+46
-35
lines changed

‎ext/POSIX/t/wrappers.t‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ SKIP: {
164164

165165
is(-e NOT_HERE, undef, NOT_HERE . ' does not exist');
166166

167-
foreach ([undef, 0, 'chdir', NOT_HERE],
167+
foreach ([undef, !!0, 'chdir', NOT_HERE],
168168
[undef, 0, 'chmod', 0, NOT_HERE],
169169
['d_chown', 0, 'chown', 0, 0, NOT_HERE],
170170
[undef, undef, 'creat', NOT_HERE . '/crash', 0],

‎lib/B/Op_private.pm‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎opcode.h‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pp_sys.c‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3902,7 +3902,7 @@ PP(pp_fttext)
39023902

39033903
PP_wrapped(pp_chdir, MAXARG, 0)
39043904
{
3905-
dSP; dTARGET;
3905+
dSP;
39063906
const char *tmps = NULL;
39073907
GV *gv = NULL;
39083908
/* pp_coreargs pushes a NULL to indicate no args passed to
@@ -3918,9 +3918,8 @@ PP_wrapped(pp_chdir, MAXARG, 0)
39183918
"chdir() on unopened filehandle %" SVf, sv);
39193919
}
39203920
SETERRNO(EBADF,RMS_IFI);
3921-
PUSHs(&PL_sv_zero);
39223921
TAINT_PROPER("chdir");
3923-
RETURN;
3922+
RETPUSHNO;
39243923
}
39253924
}
39263925
else if (!(gv = MAYBE_DEREF_GV(sv)))
@@ -3941,10 +3940,9 @@ PP_wrapped(pp_chdir, MAXARG, 0)
39413940
tmps = SvPV_nolen_const(*svp);
39423941
}
39433942
else {
3944-
PUSHs(&PL_sv_zero);
39453943
SETERRNO(EINVAL, LIB_INVARG);
39463944
TAINT_PROPER("chdir");
3947-
RETURN;
3945+
RETPUSHNO;
39483946
}
39493947
}
39503948

@@ -3954,13 +3952,13 @@ PP_wrapped(pp_chdir, MAXARG, 0)
39543952
IO* const io = GvIO(gv);
39553953
if (io) {
39563954
if (IoDIRP(io)) {
3957-
PUSHi(fchdir(my_dirfd(IoDIRP(io))) >= 0);
3955+
PUSHs(boolSV(fchdir(my_dirfd(IoDIRP(io))) >= 0));
39583956
} else if (IoIFP(io)) {
39593957
int fd = PerlIO_fileno(IoIFP(io));
39603958
if (fd < 0) {
39613959
goto nuts;
39623960
}
3963-
PUSHi(fchdir(fd) >= 0);
3961+
PUSHs(boolSV(fchdir(fd) >= 0));
39643962
}
39653963
else {
39663964
goto nuts;
@@ -3974,7 +3972,7 @@ PP_wrapped(pp_chdir, MAXARG, 0)
39743972
#endif
39753973
}
39763974
else
3977-
PUSHi( PerlDir_chdir(tmps) >= 0 );
3975+
PUSHs(boolSV( PerlDir_chdir(tmps) >= 0 ));
39783976
#ifdef VMS
39793977
/* Clear the DEFAULT element of ENV so we'll get the new value
39803978
* in the future. */
@@ -3986,8 +3984,7 @@ PP_wrapped(pp_chdir, MAXARG, 0)
39863984
nuts:
39873985
report_evil_fh(gv);
39883986
SETERRNO(EBADF,RMS_IFI);
3989-
PUSHs(&PL_sv_zero);
3990-
RETURN;
3987+
RETPUSHNO;
39913988
#endif
39923989
}
39933990

‎regen/opcodes‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ ftbinary -B ck_ftst isu- F-
435435
# File calls.
436436

437437
# chdir really behaves as if it had both "S?" and "F?"
438-
chdir chdir ck_trunc isT% S?
438+
chdir chdir ck_trunc is% S?
439439
chown chown ck_fun imsT@ L
440440
chroot chroot ck_fun isTu% S?
441441
unlink unlink ck_fun imsTu@ L

‎t/op/chdir.t‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@ BEGIN {
1212
set_up_inc(qw(t . lib ../lib));
1313
}
1414

15-
plan(tests => 44);
15+
plan(tests => 2 + 20 + 1 + 1 + 3*8 + 3);
1616

1717
use Config;
1818
use Errno qw(ENOENT EBADF EINVAL);
19+
no warnings qw(experimental::builtin); # is_bool
1920

2021
my $IsVMS = $^O eq 'VMS';
2122

@@ -53,7 +54,7 @@ SKIP: {
5354
$Cwd = abs_path;
5455

5556
SKIP: {
56-
skip("no fchdir", 19) unless $has_fchdir;
57+
skip("no fchdir", 20) unless $has_fchdir;
5758
my $has_dirfd = ($Config{d_dirfd} || $Config{d_dir_dd_fd} || "") eq "define";
5859
ok(opendir(my $dh, "."), "opendir .");
5960
ok(open(my $fh, "<", "op"), "open op");
@@ -92,7 +93,9 @@ SKIP: {
9293
{
9394
my $warn;
9495
local $SIG{__WARN__} = sub { $warn = shift };
95-
ok(!chdir(H), "check we can't chdir to closed handle");
96+
my $r = chdir(H);
97+
ok(!$r, "check we can't chdir to closed handle");
98+
ok(builtin::is_bool($r), 'chdir returns bool on failure');
9699
is(0+$!, EBADF, 'check $! set appropriately');
97100
like($warn, qr/on closed filehandle H/, 'like closed');
98101
$! = 0;
@@ -122,20 +125,25 @@ sub check_env {
122125

123126
# Make sure $ENV{'SYS$LOGIN'} is only honored on VMS.
124127
if( $key eq 'SYS$LOGIN' && !$IsVMS ) {
125-
ok( !chdir(), "chdir() on $^O ignores only \$ENV{$key} set" );
126-
is( abs_path, $Cwd, ' abs_path() did not change' );
127-
pass( " no need to test SYS\$LOGIN on $^O" ) for 1..4;
128+
my $r = chdir();
129+
ok( !$r, "chdir() on $^O ignores only \$ENV{$key} set" );
130+
ok( builtin::is_bool($r), ' and the return value is a bool' );
131+
is( abs_path, $Cwd, ' abs_path() did not change' );
132+
pass( " no need to test SYS\$LOGIN on $^O" ) for 1..5;
128133
}
129134
else {
130135
ok( chdir(), "chdir() w/ only \$ENV{$key} set" );
131136
is( abs_path, $ENV{$key}, ' abs_path() agrees' );
132-
chdir($Cwd);
137+
my $r = chdir($Cwd);
133138
is( abs_path, $Cwd, ' and back again' );
139+
ok( builtin::is_bool($r), ' and the return value is a bool' );
134140

135141
my $warning = '';
136142
local $SIG{__WARN__} = sub { $warning .= join '', @_ };
137143
$! = 0;
138-
ok(!chdir(''), "chdir('') no longer implied chdir()");
144+
$r = chdir('');
145+
ok(!$r, "chdir('') no longer implies chdir()");
146+
ok(builtin::is_bool($r), 'chdir returns bool on failure');
139147
is($!+0, ENOENT, 'check $! set appropriately');
140148
is($warning, '', 'should no longer warn about deprecation');
141149
}

‎t/op/coreamp.t‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -410,21 +410,27 @@ use if !is_miniperl, File::Spec::Functions, qw(curdir);
410410
test_proto 'chdir';
411411
unless (is_miniperl) {
412412
$tests += 7;
413+
my ($false, $true) = (!!0, !!1);
413414
my $good_dir = curdir();
414415
my $bad_dir = 'no_such_dir+*?~';
415-
is mychdir($good_dir), 1, 'mychdir(".") succeeds';
416-
is mychdir($bad_dir), 0, 'mychdir($bad_dir) fails';
417-
is &CORE::chdir($good_dir), 1, '&chdir(".") succeeds';
418-
is &CORE::chdir($bad_dir), 0, '&chdir($bad_dir) fails';
416+
is mychdir($good_dir), $true, 'mychdir(".") succeeds';
417+
is mychdir($bad_dir), $false, 'mychdir($bad_dir) fails';
418+
is &CORE::chdir($good_dir), $true, '&chdir(".") succeeds';
419+
is &CORE::chdir($bad_dir), $false, '&chdir($bad_dir) fails';
419420
{
420421
local $ENV{HOME} = $good_dir;
421-
is &CORE::chdir(), 1, '&chdir() succeeds with $ENV{HOME} = "."';
422+
is &CORE::chdir(), $true, '&chdir() succeeds with $ENV{HOME} = "."';
422423
$ENV{HOME} = $bad_dir;
423-
is &CORE::chdir(), 0, '&chdir() fails with $ENV{HOME} = $bad_dir';
424+
is &CORE::chdir(), $false, '&chdir() fails with $ENV{HOME} = $bad_dir';
424425
}
425-
{
426+
SKIP: {
427+
# I don't know enough about VMS to tell whether it is possible to
428+
# delete $ENV{'SYS$LOGIN'} and what that would mean, so just be
429+
# cautious and skip this test there until someone can verify.
430+
skip 'not messing with SYS$LOGIN on VMS', 1
431+
if $^O eq 'VMS';
426432
delete local @ENV{qw(HOME LOGDIR SYS$LOGIN)};
427-
is &CORE::chdir(), 0, '&chdir() fails with @ENV{qw(HOME LOGDIR SYS$LOGIN)} unset';
433+
is &CORE::chdir(), $false, '&chdir() fails with @ENV{qw(HOME LOGDIR SYS$LOGIN)} unset';
428434
}
429435
}
430436

0 commit comments

Comments
 (0)