On 5/15/20 16:45, Gabriel Ivăncescu wrote:
Hi Paul,
I've sent a patch (multiple times since it hasn't been reviewed, unfortunately) for the same bug and, while it didn't add the (trivial) tests, I believe it handled some extra corner cases, which you might want to add.
See: https://source.winehq.org/patches/data/184165
In particular, I'm concerned what happens with your patch if the file itself is either moved, deleted, or refers to something special (like bash's pipe:[12345678] for example), since you don't test the resulted filename at all—and it might not be the same file.
Also it would be nice to add an implementation with #elif defined(F_GETPATH) for other Unix platforms or MacOS (you can look at my patch for trivial example, but I haven't tested it).
And lastly we have a bug on bugzilla about it already:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46070
Thanks, Gabriel
Oh yeah, I missed that patch entirely, sorry. I know about Macos F_GETPATH but I did not add that because I don't have a Mac to test. Regarding the other thing, yes, a lot of things can go wrong, but I am not sure we want or can really support all that cases. There is actually numerous ways how the Python error can be fixed. E. g., we can make GetFileInformationByHandle not to fail by not requiesting the full attributes from NtQueryAttributes file. After a lot of surfing around the code I was inclined to choose the way you did as it seems that it is touching the core reason. But maybe we should really avoid touching that and instead change GetFileInformationByHandle. Or make such file something special, so GetFileType() won't return FILE_TYPE_DISK for it (maybe it is better really; Python is happy this way either). Python is also fine if we don't have valid std handles. Or, instead, do something with the code which makes handles from unix fds. I guess that was added to allow piping native and line command line programs. Maybe it is possible to change wine loader some way so we avoid creating std handles this way.
So I guess we need some discussion on general direction before tweaking these patches.