Project

General

Profile

Actions

Bug #3142

closed

error: use of undeclared identifier 'COPYFILE_CLONE_FORCE'

Added by ryandesign 8 months ago. Updated 8 months ago.

Status:
Fixed
Priority:
Normal
Category:
mod_webdav
Target version:
ASK QUESTIONS IN Forums:
No

Description

lighttpd 1.4.64 (with the fix from #3140) fails to build on OS X 10.11 and earlier:

mod_webdav.c:2597:58: error: use of undeclared identifier 'COPYFILE_CLONE_FORCE'
        if (0==copyfile(src->path.ptr,dst->path.ptr,NULL,COPYFILE_CLONE_FORCE))
                                                         ^
mod_webdav.c:2645:59: error: use of undeclared identifier 'COPYFILE_CLONE_FORCE'
        if (0 == copyfile(src->path.ptr, tmpb->ptr, NULL, COPYFILE_CLONE_FORCE))
                                                          ^
2 errors generated.

https://build.macports.org/builders/ports-10.11_x86_64-builder/builds/171342/steps/install-port/logs/stdio

Actions #1

Updated by gstrauss 8 months ago

Reference:

https://www.unix.com/man-page/mojave/3/copyfile/

HISTORY

     The copyfile() API was introduced in Mac OS X 10.5.

lighttpd checks for OS X 10.5+

      #if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101200 /* 10.12+ */
        if (0==clonefile(src->path.ptr,dst->path.ptr,CLONE_NOFOLLOW))
      #else /* __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050 */
        if (0==copyfile(src->path.ptr,dst->path.ptr,NULL,COPYFILE_CLONE_FORCE))
      #endif

Is lighttpd code above not checking for 10.5 correctly? (I believe the check is correct)
Is the manual page wrong?
Is COPYFILE_CLONE defined?
Is (COPYFILE_EXCL | COPYFILE_ACL | COPYFILE_STAT | COPYFILE_XATTR | COPYFILE_DATA | COPYFILE_NOFOLLOW_SRC) defined?

Actions #2

Updated by ryandesign 8 months ago

Lighttpd isn't checking for 10.5+. It's checking if 10.12+ then use clonefile else use copyfile.

When using copyfile, it's using COPYFILE_CLONE_FORCE, but COPYFILE_CLONE_FORCE appears to have been introduced in 10.12. I can't find any reference for that, I just see that COPYFILE_CLONE_FORCE is defined in /usr/include/copyfile.h in the MacOSX10.12.sdk while COPYFILE_CLONE_FORCE is not present in any file in /usr/include in the MacOSX10.11.sdk.

The 10.11 SDK has:

$ grep -E '(COPYFILE_CLONE|COPYFILE_EXCL|COPYFILE_ACL|COPYFILE_STAT|COPYFILE_XATTR|COPYFILE_DATA|COPYFILE_NOFOLLOW_SRC)\b' /usr/include/copyfile.h
#define COPYFILE_ACL        (1<<0)
#define COPYFILE_STAT        (1<<1)
#define COPYFILE_XATTR        (1<<2)
#define COPYFILE_DATA        (1<<3)
#define COPYFILE_SECURITY   (COPYFILE_STAT | COPYFILE_ACL)
#define COPYFILE_METADATA   (COPYFILE_SECURITY | COPYFILE_XATTR)
#define COPYFILE_ALL        (COPYFILE_METADATA | COPYFILE_DATA)
#define COPYFILE_EXCL        (1<<17) /* fail if destination exists */
#define COPYFILE_NOFOLLOW_SRC    (1<<18) /* don't follow if source is a symlink */
#define COPYFILE_NOFOLLOW    (COPYFILE_NOFOLLOW_SRC | COPYFILE_NOFOLLOW_DST)
Actions #3

Updated by ryandesign 8 months ago

This copyfile manpage has the same modification date as the one on 10.11 so it could be used as a reference:

https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/copyfile.3.html

Actions #4

Updated by gstrauss 8 months ago

From an earlier manual page from 10.6.2, COPYFILE_CLONE and COPYFILE_CLONE_FORCE are not defined.
(That's crap that the manual pages did not indicate the versions where additional flags were introduced)
https://www.unix.com/man-page/osx/3/copyfile/

--- a/src/mod_webdav.c
+++ b/src/mod_webdav.c
@@ -2588,6 +2588,11 @@ webdav_copytmp_rename (const plugin_config * const pconf,
 {
   #if defined(__APPLE__) && defined(__MACH__)
   #if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
+   #ifndef COPYFILE_CLONE_FORCE
+   #define COPYFILE_CLONE_FORCE \
+     (COPYFILE_EXCL  | COPYFILE_ACL  | COPYFILE_STAT \
+     |COPYFILE_XATTR | COPYFILE_DATA | COPYFILE_NOFOLLOW_SRC)
+   #endif
     if (!(*flags & (WEBDAV_FLAG_COPY_XDEV
                    |WEBDAV_FLAG_MOVE_XDEV
                    |WEBDAV_FLAG_NO_CLONE))) {

Do you have a preference between the above patch and me modifying the code to disable copyfile() for <= OS X 10.11, and instead using fcopyfile(ifd, ofd, NULL, COPYFILE_ALL) (already used as a fallback in lighttpd mod_webdav)?

I am leaning towards the latter:

--- a/src/mod_webdav.c
+++ b/src/mod_webdav.c
@@ -2587,7 +2587,7 @@ webdav_copytmp_rename (const plugin_config * const pconf,
                        int * const flags)
 {
   #if defined(__APPLE__) && defined(__MACH__)
-  #if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
+  #if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101200 /* 10.12+ */
     if (!(*flags & (WEBDAV_FLAG_COPY_XDEV
                    |WEBDAV_FLAG_MOVE_XDEV
                    |WEBDAV_FLAG_NO_CLONE))) {
@@ -2635,7 +2635,7 @@ webdav_copytmp_rename (const plugin_config * const pconf,
   do {

    #if defined(__APPLE__) && defined(__MACH__)
-   #if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
+   #if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101200 /* 10.12+ */
     if (!(*flags & (WEBDAV_FLAG_COPY_XDEV
                    |WEBDAV_FLAG_MOVE_XDEV
                    |WEBDAV_FLAG_NO_CLONE))) {

Actions #5

Updated by gstrauss 8 months ago

  • Category set to mod_webdav
  • Status changed from New to Patch Pending
  • Target version changed from 1.4.xx to 1.4.65
Actions #6

Updated by ryandesign 8 months ago

I don't have a preference because I'm not familar with these functions or how they're used within lighttpd.

Actions #7

Updated by gstrauss 8 months ago

  • Status changed from Patch Pending to Fixed
Actions #8

Updated by gstrauss 8 months ago

You can see in the full patch where lighttpd was checking for 10.5+ before clonefile() or copyfile()

I don't have a preference because I'm not familar with these functions or how they're used within lighttpd.

FYI: These are on fallback paths in lighttpd when using WebDAV COPY or MOVE and lighttpd was unable to use link() or rename() due to user mod_webdav configuration or due to crossing filesystem boundaries.

Actions #9

Updated by ryandesign 8 months ago

Ah yes, I see now when I look at the full code that it does check properly for 10.5+. That was not evident from the snippet you posted.

I'm about to apply your patch to MacPorts until the next release.

In our ticket, Josh had this to add:

I don't think the man page can possibly be correct about COPYFILE_EXCL | COPYFILE_ACL | COPYFILE_STAT | COPYFILE_XATTR | COPYFILE_DATA | COPYFILE_NOFOLLOW_SRC being equivalent to COPYFILE_CLONE_FORCE, because it says that for both COPYFILE_CLONE_FORCE and COPYFILE_CLONE, and because they're not actually defined that way in the header, they each set a distinct bit:

#define COPYFILE_CLONE        (1<<24)
#define COPYFILE_CLONE_FORCE    (1<<25)

So I think if the intent is to clone the file and fail if that's not possible, that simply can't be done on older than 10.12, and the check to detect that would be simply #ifdef COPYFILE_CLONE_FORCE.
Actions

Also available in: Atom