Some CRTs don't support 200 open files, resulting at least in failing tests.

[ams-tschoening]

I'm updating QPDF from 6.0 to 8.4 and one of the new features is that it automatically decides if to keep files open or not. The currently hard coded limit is 200, but my Embarcadero C++Builder only supports 50 files open at the same time for legacy 32 Bit apps. I came across that issue in my own app merging PDFs already and simply fixed it by only opening 35 files at a time at most, so don't run into actual problems with using QPDF itself.

But you have added tests based on the knowledge of the 200 open files and the following fails in my case:

$td->runtest("don't disable keep files open",
			 {$td->COMMAND =>
				  "qpdf --verbose --static-id --empty" .
				  " --pages 1*kfo.pdf -- a.pdf"},
			 {$td->FILE => "enable-kfo.out", $td->EXIT_STATUS => 0},
			 $td->NORMALIZE_NEWLINES);

The reason simply is that the used file name expands to more than 50 files and after file 47 they can't be opened anymore. This can easily be seen in the logs:

--> BEGIN ACTUAL OUTPUT <--
qpdf: selecting --keep-open-files=y
qpdf: processing 100-kfo.pdf
qpdf: processing 101-kfo.pdf
qpdf: processing 102-kfo.pdf
qpdf: processing 103-kfo.pdf
qpdf: processing 104-kfo.pdf
qpdf: processing 105-kfo.pdf
qpdf: processing 106-kfo.pdf
qpdf: processing 107-kfo.pdf
qpdf: processing 108-kfo.pdf
qpdf: processing 109-kfo.pdf
qpdf: processing 110-kfo.pdf
qpdf: processing 111-kfo.pdf
qpdf: processing 112-kfo.pdf
qpdf: processing 113-kfo.pdf
qpdf: processing 114-kfo.pdf
qpdf: processing 115-kfo.pdf
qpdf: processing 116-kfo.pdf
qpdf: processing 117-kfo.pdf
qpdf: processing 118-kfo.pdf
qpdf: processing 119-kfo.pdf
qpdf: processing 120-kfo.pdf
qpdf: processing 121-kfo.pdf
qpdf: processing 122-kfo.pdf
qpdf: processing 123-kfo.pdf
qpdf: processing 124-kfo.pdf
qpdf: processing 125-kfo.pdf
qpdf: processing 126-kfo.pdf
qpdf: processing 127-kfo.pdf
qpdf: processing 128-kfo.pdf
qpdf: processing 129-kfo.pdf
qpdf: processing 130-kfo.pdf
qpdf: processing 131-kfo.pdf
qpdf: processing 132-kfo.pdf
qpdf: processing 133-kfo.pdf
qpdf: processing 134-kfo.pdf
qpdf: processing 135-kfo.pdf
qpdf: processing 136-kfo.pdf
qpdf: processing 137-kfo.pdf
qpdf: processing 138-kfo.pdf
qpdf: processing 139-kfo.pdf
qpdf: processing 140-kfo.pdf
qpdf: processing 141-kfo.pdf
qpdf: processing 142-kfo.pdf
qpdf: processing 143-kfo.pdf
qpdf: processing 144-kfo.pdf
qpdf: processing 145-kfo.pdf
qpdf: processing 146-kfo.pdf
qpdf: processing 147-kfo.pdf
open 147-kfo.pdf: Error 0

--> END ACTUAL OUTPUT <--

It's 50 files minus STD* and stuff, so breaking on file 147-kfo.pdf makes sense to me. If I simply change the files to 11*kfo.pdf instead, reducing the number to 10, and after adopting the expected output as well, the test passes. I'll additionally provide a PR implementing this later as well.

I would be very thankful if you could address this issue somehow. I currently preferred changing the test simply because in your two other tests you are only using a subset of those 200 possible files as well and I need to get things going.

But because your chosen limit of 200 is some arbitrary value anyway, how about reducing it to a safer value like 25 or 35 like I use? That would need adoption of manuals etc. as well.

Checking things for the current runtime wouldn't help me solve this problem necessarily as well, because like can be read in my link to SO, my compiler lacks lots of functions suggested for this purpose. And I'm not sure what of the things my compiler provides is usable for others.

So I guess it's either changing the tests only or simply reducing the hard coded value. Thanks!

[ams-tschoening]

Just for reference the code I'm currently using to decide how many open files to support in my own app:

#include <algorithm>
typedef std::list<QpdfPtr> AddedPdfs;

AddedPdfs::size_type	maxPdfsToAddMin(std::min(FOPEN_MAX,		35));
AddedPdfs::size_type	maxPdfsToAdd(	std::max(FOPEN_MAX / 2, (int) maxPdfsToAddMin));

[jberkenbilt]

I can definitely add a flag to set the threshold. I probably should have done that to begin with. Somehow I managed to not know about FOPEN_MAX. Thanks for pointing that out. I'll take care of this for my next update to qpdf. It should be a small change. I will change the default to be based on FOPEN_MAX with a cap and will add a command-line flag to change the threshold. I'll look at the test suite and see if I can make it work on systems with smaller maxes than 200 without breaking the intended functionality of the test. A PR is welcome but not required. I doubt it would take me more than a few minutes to address this, though I won't be able to get to it for a bit since I don't have time to work on qpdf most of the time and just take reserved blocks of time for it every few months.

[ams-tschoening]

Just don't hurry using FOPEN_MAX, I have no idea which compilers support that. :-) If you really want to make the threshold configurable using a flag, I suggest simply reusing --keep-files-open=...: y|n|35|200|...

[jberkenbilt]

I see no real downside to your test change, but I think a better fix is required, such as adding a numeric option to --keep-files-open as you suggested and possibly seeing if FOPEN_MAX is standard or if there's another way. So I'll reopen this issue pending a more complete fix.

[jberkenbilt]

By the way @ams-tschoening thanks for all your pull requests!

[ams-tschoening]

Thanks for merging! I'm the one still sitting on the broken compiler. :-)

[jberkenbilt]

My solution is to add a --keep-files-open-threshold=count parameter. I have also updated the test suite to test with 50 open files. Hopefully this will be sufficient to both enable you to run the test suite and actually work with this in your environment.

[ams-tschoening]

Note for myself: I had expected tests to fail with 50 open files at the same time, because from my understanding the C-lib only supported 47 file handles, because of STDIN, *OUT and *ERR being already open. Doesn't seem so for this case currently, but if things fail in future, simply suggest reducing the number to 45 or such again.

Overall test suite        [...]... PASSED

TESTS COMPLETE.  Summary:

Total tests: 2847
Passes: 2844
Failures: 0
Unexpected Passes: 0
Expected Failures: 3
Missing Tests: 0
Extra Tests: 0