Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Cygwin build #106

Draft
wants to merge 18 commits into
base: edge
Choose a base branch
from
Draft

Fix Cygwin build #106

wants to merge 18 commits into from

Conversation

CodingKoopa
Copy link

@CodingKoopa CodingKoopa commented Mar 8, 2024

I was interested in using ntfsusermap on Windows but the current source wouldn't build on Cygwin. After some changes I got it to work. This PR just has fixes for Cygwin; I looked into MinGW too (over at the mingw branch) but it was too much work.

This PR is based on #105 (partly because I thought the timespec fix would help me). From there, each commit fixes a compiler/linker error (or set of errors). I don't really have the time or motivation to see this through to the end, so I may need to close the PR if nontrivial changes are needed. At the very least, I wanted to contribute some tips for anyone else who wanted to do the same. And, perhaps there are some fixes worth cherry-picking in isolation ^^

The changes range from well-reasoned fixes like the ntdll linking in d43ede9, to quick hacks like f196023. It's a mixed bag, sorry!

WinterMute and others added 15 commits February 24, 2024 09:39
This conflicts with the GUID defined in the Windows headers, and we don't seem to use this one.

Fixes compiler error (could also be fixed by redefining Windows' GUID):

win32_io.c:55:3: error: conflicting types for 'GUID'; have 'struct <anonymous>'
   55 | } GUID;
      |   ^~~~
In file included from /usr/include/w32api/winnt.h:648,
                 from /usr/include/w32api/minwindef.h:163,
                 from /usr/include/w32api/windef.h:9,
                 from /usr/include/w32api/windows.h:69,
                 from win32_io.c:31:
/usr/include/w32api/guiddef.h:24:3: note: previous declaration of 'GUID' with type 'GUID'
   24 | } GUID;
      |   ^~~~
Fixes compiler error:

In file included from ../include/ntfs-3g/logging.h:34,
                 from ../include/ntfs-3g/debug.h:29,
                 from win32_io.c:73:
../include/ntfs-3g/types.h:114:3: error: conflicting types for 'BOOL'; have 'enum <anonymous>'
  114 | } BOOL;
      |   ^~~~
In file included from /usr/include/w32api/windef.h:9,
                 from /usr/include/w32api/windows.h:69,
                 from win32_io.c:31:
/usr/include/w32api/minwindef.h:131:15: note: previous declaration of 'BOOL' with type 'BOOL' {aka 'int'}
  131 |   typedef int BOOL;
      |               ^~~~

This leverages the fact that, in minwindef.h, the definition is guarded like so:

#if !defined(__OBJC__) && !defined(__OBJC_BOOL) && !defined(__objc_INCLUDE_GNU) && !defined(_NO_BOOL_TYPEDEF)
  typedef int BOOL;
#endif
Some of the STATUS enum constants conflict with those from winnt.h.
This change undefines all of them to be safe.

Fixes compiler error:

In file included from /usr/include/w32api/minwindef.h:163,
                 from /usr/include/w32api/windef.h:9,
                 from /usr/include/w32api/windows.h:69,
                 from win32_io.c:31:
win32_io.c:132:4: error: expected identifier before '(' token
  132 |    STATUS_INVALID_HANDLE =       0xC0000008,
      |    ^~~~~~~~~~~~~~~~~~~~~
bcrypt.h defines this as a long, which causes a conflict. On LLP64, long and
u32 are the same size, but have different signedness. Signed seems to be
the prevailing view of the type.

Fixes compiler error:

win32_io.c:174:13: error: conflicting types for 'NTSTATUS'; have 'u32' {aka 'unsigned int'}
  174 | typedef u32 NTSTATUS; /* do not let the compiler choose the size */
      |             ^~~~~~~~
In file included from /usr/include/w32api/wincrypt.h:846,
                 from /usr/include/w32api/windows.h:95,
                 from win32_io.c:31:
/usr/include/w32api/bcrypt.h:27:16: note: previous declaration of 'NTSTATUS' with type 'NTSTATUS' {aka 'int'}
   27 |   typedef LONG NTSTATUS,*PNTSTATUS;
      |                ^~~~~~~~
Fixes compiler error (among others):

win32_io.c: In function 'ntfs_device_win32_stat':
win32_io.c:1804:31: error: invalid application of 'sizeof' to incomplete type 'struct stat64'
 1804 |         memset(buf, 0, sizeof(struct stat));
      |                               ^~~~~~
Fixes compiler error:

win32_io.c:2037:5: error: conflicting types for 'ntfs_device_win32_ftruncate'; have 'int(struct ntfs_device *, s64)' {aka 'int(struct ntfs_device *, long int)'}
 2037 | int ntfs_device_win32_ftruncate(struct ntfs_device *dev, s64 size)
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../include/ntfs-3g/device.h:30,
                 from win32_io.c:77:
../include/ntfs-3g/device_io.h:75:5: note: previous declaration of 'ntfs_device_win32_ftruncate' with type 'int(struct ntfs_device *, s64)' {aka 'int(struct ntfs_device *, long int)'}
   75 | int ntfs_device_win32_ftruncate(struct ntfs_device*, s64);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
_get_osfhandle is what is documented. The alternative might have been removed.

Fixes linker error:

/cygdrive/e/Documents/Programs/ntfs-3g/libntfs-3g/win32_io.c:1990:(.text+0x21a5): undefined reference to `get_osfhandle'
/usr/lib/gcc/x86_64-pc-cygwin/11/../../../../x86_64-pc-cygwin/bin/ld: .libs/libntfs_3g_la-win32_io.o: in function `ntfs_win32_ftruncate':
/cygdrive/e/Documents/Programs/ntfs-3g/libntfs-3g/win32_io.c:2054:(.text+0x223a): undefined reference to `get_osfhandle'
win32_io.c uses some symbols from this DLL.

Fixes linker error (among others):

/usr/lib/gcc/x86_64-pc-cygwin/11/../../../../x86_64-pc-cygwin/bin/ld: .libs/libntfs_3g_la-win32_io.o: in function `ntfs_device_win32_pio':
/cygdrive/e/Documents/Programs/ntfs-3g/libntfs-3g/win32_io.c:1386:(.text+0x274): undefined reference to `NtWriteFile'
/usr/lib/gcc/x86_64-pc-cygwin/11/../../../../x86_64-pc-cygwin/bin/ld: /cygdrive/e/Documents/Programs/ntfs-3g/libntfs-3g/win32_io.c:1391:(.text+0x348): undefined reference to `NtReadFile'
It might be worth figuring out a more proper fix than this.

Fixes compiler error:

In file included from ntfswipe.c:63:
ntfswipe.c: In function ‘version’:
utils.h:113:58: error: expected expression before ‘)’ token
  113 |                 ntfs_utils_reformat(_b,MAX_FMT,fmt), args); } while (0)
      |                                                          ^
../include/ntfs-3g/logging.h:95:40: note: in expansion of macro ‘ntfs_log_redirect’
   95 | #define ntfs_log_info(FORMAT, ARGS...) ntfs_log_redirect(__FUNCTION__,__FILE__,__LINE__,NTFS_LOG_LEVEL_INFO,NULL,FORMAT,##ARGS)
      |                                        ^~~~~~~~~~~~~~~~~
Copied from a header in the public domain.

Fixes compiler error:

ntfsusermap.c:149:13: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘LookupAccountNameA’
  149 | BOOL WINAPI LookupAccountNameA(const char*, const char*, void*,
      |             ^~~~~~~~~~~~~~~~~~
ntfsusermap.c:151:13: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘GetUserNameA’
  151 | BOOL WINAPI GetUserNameA(char*, u32*);
      |             ^~~~~~~~~~~~
The desired mkdir does not seem to exist on Cygwin.

Fixes compiler error:

ntfsusermap.c: In function ‘outputmap’:
ntfsusermap.c:817:25: error: too few arguments to function ‘mkdir’
  817 |                         mkdir(fullname);
      |                         ^~~~~
In file included from /usr/include/sys/_default_fcntl.h:212,
                 from /usr/include/sys/fcntl.h:3,
                 from /usr/include/fcntl.h:13,
                 from ntfsusermap.c:113:
/usr/include/sys/stat.h:140:9: note: declared here
  140 | int     mkdir (const char *_path, mode_t __mode );
      |         ^~~~~
Fixes linker error:

/usr/lib/gcc/x86_64-pc-cygwin/11/../../../../x86_64-pc-cygwin/bin/ld: ntfsclone.o: in function `open_image':
/cygdrive/e/Documents/Programs/ntfs-3g/ntfsprogs/ntfsclone.c:2385:(.text.startup+0x34e): undefined reference to `setmode'
This fixes a compiler error only manifesting in debug mode.
This fixes a compiler error only manifesting in debug mode.
ntfsprogs/ntfsusermap.c Outdated Show resolved Hide resolved
@@ -134,6 +127,30 @@ static LPFN_SETFILEPOINTEREX fnSetFilePointerEx = NULL;
#define FNPOSTFIX "A"
#endif

/*
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to just skip this enum when any constant is already defined (instead of doing this bunch of undef's)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may not be safe to do this. The first compiler error occurs with STATUS_INVALID_HANDLE (as opposed to STATUS_SUCCESS or STATUS_BUFFER_OVERFLOW), which suggests to me that the included header is not ntstatus.h, but rather winnt.h. The later is a rather incomplete group of status codes, missing STATUS_BUFFER_OVERFLOW, so it can't be used as-is.

What I can do, however, is include ntstatus.h, which does provide a comprehensive set of status codes. The remaining questions, then, are:

  • Should use ntstatus's defines no matter what, or still do it depending on whether STATUS_SUCCESS is defined?
  • Will this header reliable be available, or should I guard it for Cygwin-only?

I'm not aware of any non-Cygwin Windows setups that are supposed to be working, but I could take the conservative route and do it conditionally, if preferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants