[PATCH 2/2] ntdll: Replace fstat and lstat with fstatat in get_file_info.
Further simplify get_file_info by using fstatat for both path-based (lstat) and fd-based (fstat) modes of operation. Best, Erich
This sadly won't work for macOS 10.8 or 10.9. fstatat(2) was introduced to macOS in 10.10. is_mount_point() already has this problem, but that's not your fault. Chip
Are you referring to a runtime issue?, since a later SDK has been needed from wine-5.8. On Sun, Aug 29, 2021 at 12:54 AM Chip Davis <cdavis(a)codeweavers.com> wrote:
This sadly won't work for macOS 10.8 or 10.9. fstatat(2) was introduced to macOS in 10.10.
is_mount_point() already has this problem, but that's not your fault.
Chip
August 29, 2021 12:18 AM, "Dean Greer" <gcenx83(a)gmail.com (mailto:gcenx83(a)gmail.com?to=%22Dean%20Greer%22%20<gcenx83(a)gmail.com>)> wrote: Are you referring to a runtime issue?, since a later SDK has been needed from wine-5.8. Yes. That function will be weak-linked, which means that on 10.8 and 10.9 it will be NULL. Calling it without checking for NULL will cause a crash on those systems. On Sun, Aug 29, 2021 at 12:54 AM Chip Davis <cdavis(a)codeweavers.com (mailto:cdavis(a)codeweavers.com)> wrote:This sadly won't work for macOS 10.8 or 10.9. fstatat(2) was introduced to macOS in 10.10. is_mount_point() already has this problem, but that's not your fault. Chip Chip
On Sat, Aug 28, 2021 at 11:40 PM Chip Davis <cdavis(a)codeweavers.com> wrote:
August 29, 2021 12:18 AM, "Dean Greer" <gcenx83(a)gmail.com> wrote:
Are you referring to a runtime issue?, since a later SDK has been needed from wine-5.8. Yes. That function will be weak-linked, which means that on 10.8 and 10.9 it will be NULL. Calling it without checking for NULL will cause a crash on those systems.
I was obviously expecting this to not be a problem since fstatat is already used in fd_is_mount_point(), is there an existing example of how we like to work around a function that's weak-linked and possibly NULL? Best, Erich
August 30, 2021 2:32 PM, "Erich E. Hoover" <erich.e.hoover(a)gmail.com> wrote:
On Sat, Aug 28, 2021 at 11:40 PM Chip Davis <cdavis(a)codeweavers.com> wrote:
August 29, 2021 12:18 AM, "Dean Greer" <gcenx83(a)gmail.com> wrote: Are you referring to a runtime issue?, since a later SDK has been needed from wine-5.8. Yes. That function will be weak-linked, which means that on 10.8 and 10.9 it will be NULL. Calling it without checking for NULL will cause a crash on those systems.
I was obviously expecting this to not be a problem since fstatat is already used in fd_is_mount_point(), is there an existing example of how we like to work around a function that's weak-linked and possibly NULL?
See lines 92-96 of dlls/ntdll/unix/sync.c. Though in this case, I don't think that method will be acceptable here, since mach_continuous_time() is unique to macOS and is used in macOS-specific code, while fstatat(2) is a standard POSIX function used in code compiled everywhere. Chip
How badly is this function needed as there is a third-party solution that implements this and more missing functions. On Mon, Aug 30, 2021 at 3:32 PM Erich E. Hoover <erich.e.hoover(a)gmail.com> wrote:
On Sat, Aug 28, 2021 at 11:40 PM Chip Davis <cdavis(a)codeweavers.com> wrote:
August 29, 2021 12:18 AM, "Dean Greer" <gcenx83(a)gmail.com> wrote:
Are you referring to a runtime issue?, since a later SDK has been needed from wine-5.8. Yes. That function will be weak-linked, which means that on 10.8 and 10.9 it will be NULL. Calling it without checking for NULL will cause a crash on those systems.
I was obviously expecting this to not be a problem since fstatat is already used in fd_is_mount_point(), is there an existing example of how we like to work around a function that's weak-linked and possibly NULL?
Best, Erich
On Mon, Aug 30, 2021 at 1:53 PM Dean Greer <gcenx83(a)gmail.com> wrote:
How badly is this function needed as there is a third-party solution that implements this and more missing functions.
We're currently using (even before this patch) the fstatat() function to detect if crossing a symlink puts us on a different drive. I'd definitely prefer for this to be "not our problem" by either using a library to fill in these routines or upping the minimum supported Mac OS X version. That said, it's not that hard on Mac OS X to implement a replacement for fstatat() using fcntl(F_GETPATH). Best, Erich
Wine was using the function for a long time but was avoided on macOS up until wine-5.8 when the old buildbot was taken out of commission. With how much of a PITA it is setting up a build environment for compiling wine on MacOSX10.8/10.9 I’d leave this as not our problem, it’s simply a matter of using the replacement library to fill in the gaps avoiding even needing a newer SDK. On Mon, Aug 30, 2021 at 4:20 PM Erich E. Hoover <erich.e.hoover(a)gmail.com> wrote:
On Mon, Aug 30, 2021 at 1:53 PM Dean Greer <gcenx83(a)gmail.com> wrote:
How badly is this function needed as there is a third-party solution that implements this and more missing functions.
We're currently using (even before this patch) the fstatat() function to detect if crossing a symlink puts us on a different drive. I'd definitely prefer for this to be "not our problem" by either using a library to fill in these routines or upping the minimum supported Mac OS X version. That said, it's not that hard on Mac OS X to implement a replacement for fstatat() using fcntl(F_GETPATH).
Best, Erich
On 8/28/21 11:53 PM, Chip Davis wrote:
This sadly won't work for macOS 10.8 or 10.9. fstatat(2) was introduced to macOS in 10.10.
is_mount_point() already has this problem, but that's not your fault.
Gijs and Dean proposed in [1] to bump the minimum required macOS version to 10.13. Is there a good reason to support versions older than 10.10? [1] https://www.winehq.org/pipermail/wine-devel/2021-July/191504.html
We’d proposed that for the prebuilt packages as it’s PITA to build libraries that are compatible with the lower versions of macosx, however the library I mentioned to you prior already covers this and more missing functions. On Mon, Aug 30, 2021 at 3:48 PM Zebediah Figura (she/her) < zfigura(a)codeweavers.com> wrote:
On 8/28/21 11:53 PM, Chip Davis wrote:
This sadly won't work for macOS 10.8 or 10.9. fstatat(2) was introduced to macOS in 10.10.
is_mount_point() already has this problem, but that's not your fault.
Gijs and Dean proposed in [1] to bump the minimum required macOS version to 10.13. Is there a good reason to support versions older than 10.10?
[1] https://www.winehq.org/pipermail/wine-devel/2021-July/191504.html
On 8/30/21 2:54 PM, Dean Greer wrote:
We’d proposed that for the prebuilt packages as it’s PITA to build libraries that are compatible with the lower versions of macosx, however the library I mentioned to you prior already covers this and more missing functions.
Maybe I'm missing something, but compatibility libraries are a moot point if this function is available in 10.13 and we're requiring 10.13, right?
Correct the official Winehq packages will be targeting 10.13 so for that it’s a moot point. The thing is if wine still “officially” supports down to 10.8 how to handle the missing functions, I’d go with using a third party implementation that would then make needing a newer SDK a moot point. On Mon, Aug 30, 2021 at 4:36 PM Zebediah Figura (she/her) < zfigura(a)codeweavers.com> wrote:
On 8/30/21 2:54 PM, Dean Greer wrote:
We’d proposed that for the prebuilt packages as it’s PITA to build libraries that are compatible with the lower versions of macosx, however the library I mentioned to you prior already covers this and more missing functions.
Maybe I'm missing something, but compatibility libraries are a moot point if this function is available in 10.13 and we're requiring 10.13, right?
Chip, Do we need to do something to deal with this or do you feel like I can resend these patches? Best, Erich On Mon, Aug 30, 2021 at 2:40 PM Dean Greer <gcenx83(a)gmail.com> wrote:
Correct the official Winehq packages will be targeting 10.13 so for that it’s a moot point.
The thing is if wine still “officially” supports down to 10.8 how to handle the missing functions, I’d go with using a third party implementation that would then make needing a newer SDK a moot point.
On Mon, Aug 30, 2021 at 4:36 PM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 8/30/21 2:54 PM, Dean Greer wrote:
We’d proposed that for the prebuilt packages as it’s PITA to build libraries that are compatible with the lower versions of macosx, however the library I mentioned to you prior already covers this and more missing functions.
Maybe I'm missing something, but compatibility libraries are a moot point if this function is available in 10.13 and we're requiring 10.13, right?
September 20, 2021 1:19 PM, "Erich E. Hoover" <erich.e.hoover(a)gmail.com> wrote:
Chip,
Do we need to do something to deal with this or do you feel like I can resend these patches?
Well, it won't break things any more than they already are, and Dean says we can build with a compatibility layer to make this work on earlier versions. I guess nothing else needs to be done on your part, then.
Best, Erich
On Mon, Aug 30, 2021 at 2:40 PM Dean Greer <gcenx83(a)gmail.com> wrote:
Correct the official Winehq packages will be targeting 10.13 so for that it’s a moot point.
The thing is if wine still “officially” supports down to 10.8 how to handle the missing functions, I’d go with using a third party implementation that would then make needing a newer SDK a moot point.
On Mon, Aug 30, 2021 at 4:36 PM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 8/30/21 2:54 PM, Dean Greer wrote:
We’d proposed that for the prebuilt packages as it’s PITA to build libraries that are compatible with the lower versions of macosx, however the library I mentioned to you prior already covers this and more missing functions.
Maybe I'm missing something, but compatibility libraries are a moot point if this function is available in 10.13 and we're requiring 10.13, right?
Chip
participants (4)
-
Chip Davis -
Dean Greer -
Erich E. Hoover -
Zebediah Figura (she/her)