Fix for Bug #74719

[alexanderholman]

Fix for bug #74719 to allow NULL context in fopen without warning e.g.:

fopen ('/tmp/foo', 'r', false, NULL);

Replaces #3005

[nikic]

This test looks a bit too complicated for what it is testing. Did you adapt it from another test file? I'm guessing that the original test was specifically dealing with include path handling, which is not really relevant here.

The test for this change should not be much more than a successful fopen() call with null context. An absolute (__DIR__ based) path can be used to be absolutely sure that include path or cwd don't matter.

[alexanderholman]

Okay I'll update the test, and you're right I did, wasn't sure of the format so based on the other fopen tests. Do you think the change to fopen_variation4 is okay?

[nikic]

It might be better to keep all the inputs and only adjust the output in variant4, but really it's fine either way. After all those cases will be tested here.

[alexanderholman]

I'll update my test and the new ones accordingly tonight

[alexanderholman]

@nikic I have updated the test changing the expected output in the variation files, with the exception of unlink-error* where I have removed the tests, it seemed a bit wrong to have tests meant to produce errors output true, if you think it should be a change instead of deletion 0eb19a8 has the changed value.

[nikic]

Looking through the file, it looks like a bunch of other functions like mkdir() and rmdir() also use Z_PARAM_RESOURCE(zcontext) right now. I think it would be good to change all of these functions to accept a null argument at the same time, to make sure things behave consistently.

[alexanderholman]

Okay. A comment in the issue also mentioned fwrite should that be looked at too?

[nikic]

@alexanderholman The fwrite() case is a bit tricky because it's a potentially BC breaking change (currently null will be interpreted as 0 in weak typing mode, which means write nothing, while adding explicit support for null would turn it into write everything). As such, I would leave that part alone for now.

[alexanderholman]

additional tests required

[alexanderholman]

not really sure if dc0735e should be in this PR but tests aren't passing without it in AppVeyor so have included it for now. See @weltling commit 9d2662e

[nikic]

Merged as a01de10 with some more simplifications to the new test file. Thanks!