Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ubuntu 12.4 sets errno for a successful call to pathconf #12095

Closed
p5pRT opened this issue May 11, 2012 · 16 comments
Closed

ubuntu 12.4 sets errno for a successful call to pathconf #12095

p5pRT opened this issue May 11, 2012 · 16 comments

Comments

@p5pRT
Copy link

p5pRT commented May 11, 2012

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

Searchable as RT112866$

@p5pRT
Copy link
Author

p5pRT commented May 11, 2012

From @nwc10

This is blead on a machine upgraded to Ubuntu 12.4 only a few days ago
$ ./perl -Ilib -T ext/POSIX/t/sysconf.t | grep -v ^ok
1..87
not ok 5 - errno should be clear in all cases
# Failed test 'errno should be clear in all cases'
# at ext/POSIX/t/sysconf.t line 64.
# got​: 2
# expected​: 0
# $!​: No such file or directory
not ok 20 - errno should be clear in all cases
# Failed test 'errno should be clear in all cases'
# at ext/POSIX/t/sysconf.t line 64.
# got​: 2
# expected​: 0
# $!​: No such file or directory
# Looks like you failed 2 tests of 87.

Which is this​:

$ ./perl -Ilib -MPOSIX -lwe '$!=0; $a = pathconf(shift, _PC_LINK_MAX); print "\$!=$!, r=$a"' .
$!=No such file or directory, r=32000

which seems to be because Ubuntu 12.4 is a special and unique snowflake, and
sets errno on a successful call to pathconf and fpathconf.

Here's my test program​:

#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char **argv) {
  while (*++argv) {
  long result;
  int gotcha;
  int fd;

  errno = 0;
  result = pathconf(*argv, _PC_LINK_MAX);
  gotcha = errno;

  printf("_PC_LINK_MAX for '%s' is %ld, errno is %d\n",
  *argv, result, gotcha);

  fd = open(*argv, O_RDONLY);
  if (fd == -1) {
  perror(*argv);
  } else {
  errno = 0;
  result = fpathconf(fd, _PC_LINK_MAX);
  gotcha = errno;

  printf("_PC_LINK_MAX for '%s' using fd %d is %ld, errno is %d\n",
  *argv, fd, result, gotcha);
  }
  }
  return 0;
}

On that Ubuntu machine​:

$ ./pathconf /tmp ._PC_LINK_MAX for '/tmp' is 32000, errno is 2
_PC_LINK_MAX for '/tmp' using fd 3 is 32000, errno is 2
_PC_LINK_MAX for '.' is 32000, errno is 2
_PC_LINK_MAX for '.' using fd 4 is 32000, errno is 2

On a FreeBSD machine with a network mounted /home​:

$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 32767, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 32767, errno is 0
_PC_LINK_MAX for '.' is -1, errno is 22
_PC_LINK_MAX for '.' using fd 4 is -1, errno is 22

On a genuine Debian* system​:

$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 32000, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 32000, errno is 0
_PC_LINK_MAX for '.' is 32000, errno is 0
_PC_LINK_MAX for '.' using fd 4 is 32000, errno is 0

So, yes, special and unique. However, I'm told POSIX says nothing about
the value of errno after a successful call, so our tests are arguably wrong.

But, hmm, this isn't helpful.

Nicholas Clark

* well, a released Debian system. Is Ubuntu simply releasing now what Debian
  will release soon?

Perl Info

Flags:
    category=install
    severity=none

Site configuration information for perl 5.16.0:

Configured by nick at Fri May 11 15:36:41 BST 2012.

Summary of my perl5 (revision 5 version 16 subversion 0) configuration:
  Commit id: 7c54b938f6609d93f8920557d6cd0859571898c8
  Platform:
    osname=linux, osvers=3.2.0-24-generic, archname=x86_64-linux
    uname='linux aranea 3.2.0-24-generic #37-ubuntu smp wed apr 25 08:43:22 utc 2012 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.6.3', 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='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.15'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Locally applied patches:
    RC0


@INC for perl 5.16.0:
    lib
    /usr/local/lib/perl5/site_perl/5.16.0/x86_64-linux
    /usr/local/lib/perl5/site_perl/5.16.0
    /usr/local/lib/perl5/5.16.0/x86_64-linux
    /usr/local/lib/perl5/5.16.0
    .


Environment for perl 5.16.0:
    HOME=/home/nick
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/nick/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented May 11, 2012

From @jmdh

On Fri, May 11, 2012 at 08​:24​:51AM -0700, Nicholas Clark wrote​:

Here's my test program​:

On that Ubuntu machine​:

$ ./pathconf /tmp ._PC_LINK_MAX for '/tmp' is 32000, errno is 2
_PC_LINK_MAX for '/tmp' using fd 3 is 32000, errno is 2
_PC_LINK_MAX for '.' is 32000, errno is 2
_PC_LINK_MAX for '.' using fd 4 is 32000, errno is 2

On a FreeBSD machine with a network mounted /home​:

$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 32767, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 32767, errno is 0
_PC_LINK_MAX for '.' is -1, errno is 22
_PC_LINK_MAX for '.' using fd 4 is -1, errno is 22

On a genuine Debian* system​:

$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 32000, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 32000, errno is 0
_PC_LINK_MAX for '.' is 32000, errno is 0
_PC_LINK_MAX for '.' using fd 4 is 32000, errno is 0

So, yes, special and unique. However, I'm told POSIX says nothing about
the value of errno after a successful call, so our tests are arguably wrong.

But, hmm, this isn't helpful.

Nicholas Clark

* well, a released Debian system. Is Ubuntu simply releasing now what Debian
will release soon?

On up-to-date Debian unstable​:

dom@​kale​:~$ cat /etc/debian_version
wheezy/sid
dom@​kale​:~$ uname -a
Linux kale 3.2.0-2-amd64 #1 SMP Mon Apr 30 05​:20​:23 UTC 2012 x86_64 GNU/Linux
dom@​kale​:~$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 127, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 127, errno is 0
_PC_LINK_MAX for '.' is 32000, errno is 0
_PC_LINK_MAX for '.' using fd 4 is 32000, errno is 0

--
Dominic Hargreaves | http​://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)

@p5pRT
Copy link
Author

p5pRT commented May 11, 2012

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

@p5pRT
Copy link
Author

p5pRT commented May 11, 2012

From [email protected]

On Fri, May 11, 2012 at 8​:24 AM, Nicholas Clark
<perlbug-followup@​perl.org> wrote​:

This is blead on a machine upgraded to Ubuntu 12.4 only a few days ago

This is from 12.04 desktop installed fresh just yesterday.

$ uname -a
Linux zim 3.2.0-24-generic #37-Ubuntu SMP Wed Apr 25 08​:43​:22 UTC 2012
x86_64 x86_64 x86_64 GNU/Linux

$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=12.04
DISTRIB_CODENAME=precise
DISTRIB_DESCRIPTION="Ubuntu 12.04 LTS"

$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 65000, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 65000, errno is 0
_PC_LINK_MAX for '.' is 65000, errno is 0
_PC_LINK_MAX for '.' using fd 4 is 65000, errno is 0

specialer and specialer...

____
Carl

@p5pRT
Copy link
Author

p5pRT commented May 12, 2012

From @pjcj

On Fri, May 11, 2012 at 08​:24​:51AM -0700, Nicholas Clark wrote​:

This is blead on a machine upgraded to Ubuntu 12.4 only a few days ago
$ ./perl -Ilib -T ext/POSIX/t/sysconf.t | grep -v ^ok
1..87
not ok 5 - errno should be clear in all cases
# Failed test 'errno should be clear in all cases'
# at ext/POSIX/t/sysconf.t line 64.
# got​: 2
# expected​: 0
# $!​: No such file or directory
not ok 20 - errno should be clear in all cases
# Failed test 'errno should be clear in all cases'
# at ext/POSIX/t/sysconf.t line 64.
# got​: 2
# expected​: 0
# $!​: No such file or directory
# Looks like you failed 2 tests of 87.

Which is this​:

$ ./perl -Ilib -MPOSIX -lwe '$!=0; $a = pathconf(shift, _PC_LINK_MAX); print "\$!=$!, r=$a"' .
$!=No such file or directory, r=32000

which seems to be because Ubuntu 12.4 is a special and unique snowflake, and
sets errno on a successful call to pathconf and fpathconf.

Here's my test program​:

#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char **argv) {
while (*++argv) {
long result;
int gotcha;
int fd;

errno = 0;
result = pathconf\(\*argv\, \_PC\_LINK\_MAX\);
gotcha = errno;

printf\("\_PC\_LINK\_MAX for '%s' is %ld\, errno is %d\\n"\,
       \*argv\, result\, gotcha\);

fd = open\(\*argv\, O\_RDONLY\);
if \(fd == \-1\) \{
    perror\(\*argv\);
\} else \{
    errno = 0;
    result = fpathconf\(fd\, \_PC\_LINK\_MAX\);
    gotcha = errno;

    printf\("\_PC\_LINK\_MAX for '%s' using fd %d is %ld\, errno is %d\\n"\,
       \*argv\, fd\, result\, gotcha\);
\}
\}
return 0;

}

On that Ubuntu machine​:

$ ./pathconf /tmp ._PC_LINK_MAX for '/tmp' is 32000, errno is 2
_PC_LINK_MAX for '/tmp' using fd 3 is 32000, errno is 2
_PC_LINK_MAX for '.' is 32000, errno is 2
_PC_LINK_MAX for '.' using fd 4 is 32000, errno is 2

On a FreeBSD machine with a network mounted /home​:

$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 32767, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 32767, errno is 0
_PC_LINK_MAX for '.' is -1, errno is 22
_PC_LINK_MAX for '.' using fd 4 is -1, errno is 22

On a genuine Debian* system​:

$ ./pathconf /tmp .
_PC_LINK_MAX for '/tmp' is 32000, errno is 0
_PC_LINK_MAX for '/tmp' using fd 3 is 32000, errno is 0
_PC_LINK_MAX for '.' is 32000, errno is 0
_PC_LINK_MAX for '.' using fd 4 is 32000, errno is 0

So, yes, special and unique. However, I'm told POSIX says nothing about
the value of errno after a successful call, so our tests are arguably wrong.

As usual, it's more complicated than that in practice.

In general, you are not supposed to read anything into the value of
errno unless the function you called has indicated failure.

However, for some functions all possible return values can indicate
success. In this case, for some return values, it is necessary to check
errno to determine whether the value being returned is a valid
successful result or an error code. strtoul() is one such example.

And then there are functions which don't promise to set errno even on
failure. setlocale() falls into this category.

Further, there are functions which don't have a specified return value
to indicate failure, but will still set errno if they fail. In such
cases, it is necessary to set errno to 0 before calling the function and
then check errno when the function returns. strerror() is an example of
this type of function.

Now, because our implementation of $! is so close to plain errno this
all becomes relevant in Perl code too.

In the case of pathconf and fpathconf the return value is the limit, if
it exists. If there is no limit the return value is -1 and errno is
unchanged. On error -1 is returned and errno is set.

This means that we should set $! to 0 before calling f?pathconf, but we
shouldn't look at it afterwards unless the function returns undef.

Looking at ext/POSIX/t/sysconf.t in more detail, I think it is checking
more than it should. 6d7cccf is making the tests too strict.

And then there's sysconf() which is being tested in the same fashion as
f?pathconf but which has different meanings for its return values. But
that's another can of worms altogether.

I can put together a patch for this over the weekend unless anyone else
wants to beat me to it.

--
Paul Johnson - paul@​pjcj.net
http​://www.pjcj.net

@p5pRT
Copy link
Author

p5pRT commented May 13, 2012

From @nwc10

On Sat, May 12, 2012 at 02​:00​:04AM +0200, Paul Johnson wrote​:

On Fri, May 11, 2012 at 08​:24​:51AM -0700, Nicholas Clark wrote​:

Looking at ext/POSIX/t/sysconf.t in more detail, I think it is checking
more than it should. 6d7cccf is making the tests too strict.

And then there's sysconf() which is being tested in the same fashion as
f?pathconf but which has different meanings for its return values. But
that's another can of worms altogether.

I can put together a patch for this over the weekend unless anyone else
wants to beat me to it.

a) I'm busy in the garden all weekend. First dry weekend in ages
b) You seem to have a better feel for what can and can't be tested than I
  do (particularly given that I wrote the tests that are now failing.
  But they did work on *everyone*'s machine until just recently)

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented May 13, 2012

From [email protected]

This problem is caused by glibc 2.15. f?pathconf() was fixed to check
whether the file system was ext4 or not, but that requires syscalls, and
the result of those syscalls may leak out of fpathconf().

Skipping the test sounds like the only sane solution, possibly with a
note in perldelta.

Vincent.

@p5pRT
Copy link
Author

p5pRT commented May 13, 2012

From @pjcj

On Sun, May 13, 2012 at 11​:21​:05AM +0100, Nicholas Clark wrote​:

On Sat, May 12, 2012 at 02​:00​:04AM +0200, Paul Johnson wrote​:

On Fri, May 11, 2012 at 08​:24​:51AM -0700, Nicholas Clark wrote​:

Looking at ext/POSIX/t/sysconf.t in more detail, I think it is checking
more than it should. 6d7cccf is making the tests too strict.

And then there's sysconf() which is being tested in the same fashion as
f?pathconf but which has different meanings for its return values. But
that's another can of worms altogether.

I can put together a patch for this over the weekend unless anyone else
wants to beat me to it.

a) I'm busy in the garden all weekend. First dry weekend in ages
b) You seem to have a better feel for what can and can't be tested than I
do (particularly given that I wrote the tests that are now failing.
But they did work on *everyone*'s machine until just recently)

I think the best we can do with respect to the f?pathconf tests is to
make sure that the perl call doesn't die, and that the system call
doesn't fail. And it's arguable we should only be testing the former.
But since we've been testing more that this anyway, it's probably safe
to test both.

With respect to the sysconf call, I think we shouldn't test more than
that perl doesn't die. Any further testing would require different
tests based the argument being passed in. Before doing that, it's
probably worth considering the purpose of the tests. I don't think we
really want to test that POSIX has been implemented correctly, only that
our layer over it is correctly implemented.

The attached patch makes these changes. I'd very much appreciate a
number of eyes looking over it, and as many people as possible testing
it on as many systems as possible. I'm wary about such changes during
the RC phase.

Matters to consider include​:

- Does the test now pass on Ubuntu systems where it previously failed?
- I've removed the QNX specific code. Does the test pass on QNX?
- Is my arithmetic correct concerning the number of tests, specifically
  with regards to skips?

--
Paul Johnson - paul@​pjcj.net
http​://www.pjcj.net

@p5pRT
Copy link
Author

p5pRT commented May 13, 2012

From @pjcj

sysconf.t.patch
diff --git a/ext/POSIX/t/sysconf.t b/ext/POSIX/t/sysconf.t
index 65625a8..8590eef 100644
--- a/ext/POSIX/t/sysconf.t
+++ b/ext/POSIX/t/sysconf.t
@@ -33,10 +33,10 @@ my @sys_consts = check qw(
     _SC_STREAM_MAX _SC_VERSION _SC_TZNAME_MAX
 );
 
-my $tests = 2 * 3 * @path_consts +
-            2 * 3 * @path_consts_terminal +
-            2 * 3 * @path_consts_fifo +
-                3 * @sys_consts;
+my $tests = 2 * 2 * @path_consts +
+            2 * 2 * @path_consts_terminal +
+            2 * 2 * @path_consts_fifo +
+                1 * @sys_consts;
 plan $tests 
      ? (tests => $tests) 
      : (skip_all => "No tests to run on this OS")
@@ -58,17 +58,17 @@ sub _check_and_report {
     my $return_val = eval {$sub->(eval "$constant()")};
     my $errno = $!; # Grab this before anything else changes it.
     is($@, '', $description);
-    SKIP: {
-	skip "terminal constants set errno on QNX", 1
-	    if $^O eq 'nto' and $description =~ $TTY;
-	cmp_ok($errno, '==', 0, 'errno should be clear in all cases')
-	    or diag("\$!: $errno");
-    }
-    SKIP: {
-        skip "constant not implemented on $^O or no limit in effect", 1 
-            if !defined($return_val);
-        like($return_val, qr/\A(?:-?[1-9][0-9]*|0 but true)\z/,
+
+    # We can't test sysconf further without investigating the type of argument
+    # provided
+    return if $description =~ /sysconf/;
+
+    if (defined $return_val) {
+	like($return_val, qr/\A(?:-?[1-9][0-9]*|0 but true)\z/,
 	     'the returned value should be a signed integer');
+    } else {
+	cmp_ok($errno, '==', 0, 'errno should be 0 as before the call')
+	    or diag("\$!: $errno");
     }
 }
 
@@ -76,7 +76,7 @@ sub _check_and_report {
 SKIP: {
     my $fd = POSIX::open($testdir, O_RDONLY)
         or skip "could not open test directory '$testdir' ($!)",
-	  3 * @path_consts;
+	  2 * @path_consts;
 
     for my $constant (@path_consts) {
 	_check_and_report(sub { fpathconf($fd, shift) }, $constant,
@@ -93,7 +93,7 @@ for my $constant (@path_consts) {
 }
 
 SKIP: {
-    my $n = 2 * 3 * @path_consts_terminal;
+    my $n = 2 * 2 * @path_consts_terminal;
 
     -c $TTY
 	or skip("$TTY not a character file", $n);
@@ -122,11 +122,11 @@ my $fifo = "fifo$$";
 
 SKIP: {
     eval { mkfifo($fifo, 0666) }
-	or skip("could not create fifo $fifo ($!)", 2 * 3 * @path_consts_fifo);
+	or skip("could not create fifo $fifo ($!)", 2 * 2 * @path_consts_fifo);
 
   SKIP: {
       my $fd = POSIX::open($fifo, O_RDONLY | O_NONBLOCK)
-	  or skip("could not open $fifo ($!)", 3 * @path_consts_fifo);
+	  or skip("could not open $fifo ($!)", 2 * @path_consts_fifo);
 
       for my $constant (@path_consts_fifo) {
 	  _check_and_report(sub { fpathconf($fd, shift) }, $constant,
@@ -150,7 +150,7 @@ END {
 SKIP: {
     if($^O eq 'cygwin') {
         pop @sys_consts;
-        skip("No _SC_TZNAME_MAX on Cygwin", 3);
+        skip("No _SC_TZNAME_MAX on Cygwin", 1);
     }
         
 }

@p5pRT
Copy link
Author

p5pRT commented May 13, 2012

From @jkeenan

On 5/13/12 6​:50 PM, Paul Johnson wrote​:
[snip]

I think the best we can do with respect to the f?pathconf tests is to
make sure that the perl call doesn't die, and that the system call
doesn't fail. And it's arguable we should only be testing the former.
But since we've been testing more that this anyway, it's probably safe
to test both.

With respect to the sysconf call, I think we shouldn't test more than
that perl doesn't die. Any further testing would require different
tests based the argument being passed in. Before doing that, it's
probably worth considering the purpose of the tests. I don't think we
really want to test that POSIX has been implemented correctly, only that
our layer over it is correctly implemented.

The attached patch makes these changes. I'd very much appreciate a
number of eyes looking over it, and as many people as possible testing
it on as many systems as possible. I'm wary about such changes during
the RC phase.

Matters to consider include​:

- Does the test now pass on Ubuntu systems where it previously failed?
- I've removed the QNX specific code. Does the test pass on QNX?
- Is my arithmetic correct concerning the number of tests, specifically
with regards to skips?

FWIW, applying the patch on a close-to-HEAD checkout of blead works for
me on Darwin/PPC. 40 fewer tests are run, but they all PASS.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented May 14, 2012

From [email protected]

Matters to consider include​:

- Does the test now pass on Ubuntu systems where it previously failed?

It passes on my glibc-2.15 system.

@p5pRT
Copy link
Author

p5pRT commented May 14, 2012

From @jmdh

On Mon, May 14, 2012 at 12​:50​:01AM +0200, Paul Johnson wrote​:

On Sun, May 13, 2012 at 11​:21​:05AM +0100, Nicholas Clark wrote​:

On Sat, May 12, 2012 at 02​:00​:04AM +0200, Paul Johnson wrote​:

On Fri, May 11, 2012 at 08​:24​:51AM -0700, Nicholas Clark wrote​:

Looking at ext/POSIX/t/sysconf.t in more detail, I think it is checking
more than it should. 6d7cccf is making the tests too strict.

And then there's sysconf() which is being tested in the same fashion as
f?pathconf but which has different meanings for its return values. But
that's another can of worms altogether.

I can put together a patch for this over the weekend unless anyone else
wants to beat me to it.

a) I'm busy in the garden all weekend. First dry weekend in ages
b) You seem to have a better feel for what can and can't be tested than I
do (particularly given that I wrote the tests that are now failing.
But they did work on *everyone*'s machine until just recently)

I think the best we can do with respect to the f?pathconf tests is to
make sure that the perl call doesn't die, and that the system call
doesn't fail. And it's arguable we should only be testing the former.
But since we've been testing more that this anyway, it's probably safe
to test both.

With respect to the sysconf call, I think we shouldn't test more than
that perl doesn't die. Any further testing would require different
tests based the argument being passed in. Before doing that, it's
probably worth considering the purpose of the tests. I don't think we
really want to test that POSIX has been implemented correctly, only that
our layer over it is correctly implemented.

The attached patch makes these changes. I'd very much appreciate a
number of eyes looking over it, and as many people as possible testing
it on as many systems as possible. I'm wary about such changes during
the RC phase.

Matters to consider include​:

- Does the test now pass on Ubuntu systems where it previously failed?

It passes on my Debian unstable-chroot-on-ext3 system where it previously
failed.

Thanks,
Dominic.

--
Dominic Hargreaves | http​://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)

@p5pRT
Copy link
Author

p5pRT commented May 14, 2012

From @nwc10

On Mon, May 14, 2012 at 12​:50​:01AM +0200, Paul Johnson wrote​:

The attached patch makes these changes. I'd very much appreciate a
number of eyes looking over it, and as many people as possible testing
it on as many systems as possible. I'm wary about such changes during
the RC phase.

Matters to consider include​:

- Does the test now pass on Ubuntu systems where it previously failed?

Yes, seems that everyone who had reported failure now reports success.

- I've removed the QNX specific code. Does the test pass on QNX?

I have no idea.

- Is my arithmetic correct concerning the number of tests, specifically
with regards to skips?

I've not checked the skips yet. I have pushed it as smoke-me/rt112866 to see
if that reveals any errors that human eyeballs miss.

Even if it passes everywhere it's tested, we shouldn't merge it until we've
manually checked the skips..

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented May 14, 2012

From @jmdh

On Mon, May 14, 2012 at 09​:52​:46AM +0100, Nicholas Clark wrote​:

On Mon, May 14, 2012 at 12​:50​:01AM +0200, Paul Johnson wrote​:

The attached patch makes these changes. I'd very much appreciate a
number of eyes looking over it, and as many people as possible testing
it on as many systems as possible. I'm wary about such changes during
the RC phase.

Matters to consider include​:

- Does the test now pass on Ubuntu systems where it previously failed?

Yes, seems that everyone who had reported failure now reports success.

- I've removed the QNX specific code. Does the test pass on QNX?

I have no idea.

- Is my arithmetic correct concerning the number of tests, specifically
with regards to skips?

I've not checked the skips yet. I have pushed it as smoke-me/rt112866 to see
if that reveals any errors that human eyeballs miss.

Even if it passes everywhere it's tested, we shouldn't merge it until we've
manually checked the skips..

This is my attempt to check the skips​:

sub _check_and_report is responsible for​:
- if $description =~ /sysconf/​: 1 test
- otherwise​: 2 tests

The first SKIP​:
- skip 2 * @​path_consts
- if not skipped, this block calls _check_and_report once for each
  iteration over @​path_consts.
- the description does not match /sysconf/
- so this is okay

The second SKIP​:
- skip 2 * 2 * @​path_consts_terminal (in either of three different cases)
- if not skipped, this block calls _check_and_report once for each
  iteration over @​path_consts_terminal and then does so a second time.
- the description does not match /sysconf/
- so this is okay

The third SKIP​:
- skip 2 * 2 * @​path_consts_fifo
- contains the forth SKIP​:
  - skip 2 * @​path_consts_fifo
  - if not skipped, this block calls _check_and_report once for each
  iteration over @​path_consts_fifo
  - the description does not match /sysconf/
  - so this is okay
- if not skipped, this block calls _check_and_report once for each
  iteration over @​path_consts_fifo and then does so a second time.
- the description does not match /sysconf/
- so this is okay

The fifth SKIP​:
- skip 1
- pops a value off @​sys_consts
- the next part of the test calls _check_and_report for each value in
  @​sys_consts
- the description matches /sysconf/ so only one test results from each
  call
- so this is okay

I think the numbers of tests skipped is correct.

Dominic.

--
Dominic Hargreaves | http​://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)

@p5pRT
Copy link
Author

p5pRT commented May 23, 2012

From @nwc10

The patch was applied as 8a2e590, and is in 5.16.0

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented May 23, 2012

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant