Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

[adding cygwin]

On 04/29/2013 04:23 PM, Paul Eggert wrote:
> ---

> +
> +	tests: don't assume getdtablesize () <= 10000000
> +	* modules/cloexec-tests:
> +	* modules/dup2-tests:
> +	* modules/dup3-tests:
> +	* modules/nonblocking-tests:
> +	* modules/posix_spawn_file_actions_addclose-tests:
> +	* modules/posix_spawn_file_actions_adddup2-tests:
> +	* modules/posix_spawn_file_actions_addopen-tests:
> +	* modules/unistd-safer-tests:
> +	Depend on the getdtablesize module.
> +	* tests/test-cloexec.c:
> +	* tests/test-dup-safer.c:
> +	* tests/test-dup2.c:
> +	* tests/test-dup3.c:
> +	* tests/test-fcntl.c:
> +	* tests/test-nonblocking.c:
> +	* tests/test-posix_spawn_file_actions_addclose.c:
> +	* tests/test-posix_spawn_file_actions_adddup2.c:
> +	* tests/test-posix_spawn_file_actions_addopen.c:
> +	Don't assume getdtablesize () <= 10000000.

This patch causes failures on at least test-fcntl and test-dup2 on
cygwin (both 32-bit and 64-bit); there, getdtablesize() currently
returns the current runtime value, but this value starts at 256, and
automatically expands as needed at runtime up to 3200.  I think cygwin
should be patched to make getdtablesize() return a constant 3200 (rather
than the current runtime value); but meanwhile, we need to do something
in gnulib to pick a larger value for bad_fd if getdtablesize() returns a
small runtime value.  I also wonder whether cygwin's
sysconf(_SC_OPEN_MAX) should likewise be modified.

(gdb) p getdtablesize()
$5 = 256
(gdb) p dup2(0, 256)
$6 = 256
(gdb) p getdtablesize()
$7 = 288
(gdb) p dup2(0, 3200)
$8 = -1
(gdb) p getdtablesize()
$9 = 288
(gdb) p dup2(0, 3199)
$10 = 3199
(gdb) p getdtablesize()
$11 = 3200

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

[Christopher Faylor]

On Tue, Sep 24, 2013 at 12:37:26PM -0600, Eric Blake wrote:
>[adding cygwin]
>
>On 04/29/2013 04:23 PM, Paul Eggert wrote:
>> ---
>
>> +
>> +	tests: don't assume getdtablesize () <= 10000000
>> +	* modules/cloexec-tests:
>> +	* modules/dup2-tests:
>> +	* modules/dup3-tests:
>> +	* modules/nonblocking-tests:
>> +	* modules/posix_spawn_file_actions_addclose-tests:
>> +	* modules/posix_spawn_file_actions_adddup2-tests:
>> +	* modules/posix_spawn_file_actions_addopen-tests:
>> +	* modules/unistd-safer-tests:
>> +	Depend on the getdtablesize module.
>> +	* tests/test-cloexec.c:
>> +	* tests/test-dup-safer.c:
>> +	* tests/test-dup2.c:
>> +	* tests/test-dup3.c:
>> +	* tests/test-fcntl.c:
>> +	* tests/test-nonblocking.c:
>> +	* tests/test-posix_spawn_file_actions_addclose.c:
>> +	* tests/test-posix_spawn_file_actions_adddup2.c:
>> +	* tests/test-posix_spawn_file_actions_addopen.c:
>> +	Don't assume getdtablesize () <= 10000000.
>
>This patch causes failures on at least test-fcntl and test-dup2 on
>cygwin (both 32-bit and 64-bit); there, getdtablesize() currently
>returns the current runtime value, but this value starts at 256, and
>automatically expands as needed at runtime up to 3200.  I think cygwin
>should be patched to make getdtablesize() return a constant 3200 (rather
>than the current runtime value);

Why?  What does "3200" have to do with anything?  There is not supposed
to be a hard-coded upper limit.

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

[Peter Rosin]

On 2013-09-25 06:32, Christopher Faylor wrote:
> On Tue, Sep 24, 2013 at 12:37:26PM -0600, Eric Blake wrote:
>> This patch causes failures on at least test-fcntl and test-dup2 on
>> cygwin (both 32-bit and 64-bit); there, getdtablesize() currently
>> returns the current runtime value, but this value starts at 256, and
>> automatically expands as needed at runtime up to 3200.  I think cygwin
>> should be patched to make getdtablesize() return a constant 3200 (rather
>> than the current runtime value);
> 
> Why?  What does "3200" have to do with anything?  There is not supposed
> to be a hard-coded upper limit.

Go back and read the gdb session. STC in C:

#include <unistd.h>
#include <stdio.h>
#include <errno.h>
int main(void)
{
  int ret = dup2(0, 3200);
  if (ret == -1)
    printf("3200 errno %d: %s\n", errno, strerror(errno));
  else
    printf("3200 ok");

  ret = dup2(0, 3199);
  if (ret == -1)
    printf("3199 errno %d: %s\n", errno, strerror(errno));
  else
    printf("3199 ok");
  return 0;
}

output on my system:

3200 errno 9: Bad file descriptor
3199 ok


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

On 09/24/2013 10:32 PM, Christopher Faylor wrote:

>> This patch causes failures on at least test-fcntl and test-dup2 on
>> cygwin (both 32-bit and 64-bit); there, getdtablesize() currently
>> returns the current runtime value, but this value starts at 256, and
>> automatically expands as needed at runtime up to 3200.  I think cygwin
>> should be patched to make getdtablesize() return a constant 3200 (rather
>> than the current runtime value);
> 
> Why?  What does "3200" have to do with anything?  There is not supposed
> to be a hard-coded upper limit.
> 

But there IS a hard-coded limit:

http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/cygwin/dtable.h?rev=1.59&content-type=text/x-cvsweb-markup&cvsroot=src

/* Initial and increment values for cygwin's fd table */
#define NOFILE_INCR    32
/* Maximum size we allow expanding to.  */
#define OPEN_MAX_MAX (100 * NOFILE_INCR)

My point is that on all other systems, even if the dtable itself
dynamically grows, getdtablesize() returns the maximum it can grow to,
not its current size.  Cygwin's behavior is the odd man out, and is
causing grief in the gnulib test suite.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

[Christopher Faylor]

On Wed, Sep 25, 2013 at 06:05:07AM -0600, Eric Blake wrote:
>On 09/24/2013 10:32 PM, Christopher Faylor wrote:
>
>>> This patch causes failures on at least test-fcntl and test-dup2 on
>>> cygwin (both 32-bit and 64-bit); there, getdtablesize() currently
>>> returns the current runtime value, but this value starts at 256, and
>>> automatically expands as needed at runtime up to 3200.  I think cygwin
>>> should be patched to make getdtablesize() return a constant 3200 (rather
>>> than the current runtime value);
>> 
>> Why?  What does "3200" have to do with anything?  There is not supposed
>> to be a hard-coded upper limit.
>> 
>
>But there IS a hard-coded limit:
>
>http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/cygwin/dtable.h?rev=1.59&content-type=text/x-cvsweb-markup&cvsroot=src

Sorry, you're right.  I should have checked before talking.  And, it
apparently you were expecting me to check.

Since you've previously made modifications in this area, why not supply
a simple patch?

cgf

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

On 09/25/2013 09:04 AM, Christopher Faylor wrote:
> 
> Since you've previously made modifications in this area, why not supply
> a simple patch?

Ah - the joys of having write access.  Yes, I'll post a patch for
discussion.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [PATCH] tests: don't assume getdtablesize () <= 10000000

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

On 09/25/2013 10:25 AM, Eric Blake wrote:
> On 09/25/2013 09:04 AM, Christopher Faylor wrote:
>>
>> Since you've previously made modifications in this area, why not supply
>> a simple patch?
> 
> Ah - the joys of having write access.  Yes, I'll post a patch for
> discussion.

What am I getting myself into?

$ cat foo.c
#include <stdio.h
#include <unistd.h>
int main(int argc, char **argv) {
  printf ("%d ", getdtablesize());
  if (argc > 1)
    printf ("%d ", dup2 (0, getdtablesize() - 1));
  printf ("%d\n", dup2 (0, getdtablesize()));
  return 0;
}
$ gcc -o foo -Wall -g foo.c
$ ./foo
Segmentation fault (core dumped)
$ strace -o /dev/null ./foo
256 256

Heisenbug!  The very act of trying to trace it makes it disappear :(

$ ./foo 1
Segmentation fault (core dumped)
$ strace -o /dev/null ./foo 1
  22687 [main] foo 2020 open_stackdumpfile: Dumping stack trace to
foo.exe.stackdump

probably some off-by-one at a boundary case, but not as simple as I had
originally hoped it would be.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]