#1612 Handle HAVE_FSEEKO only in one place.

[Carlo Bramini]

When compiling my port for Windows Embedded, I discovered that there are several sources that implements an #ifdef HAVE_FSEEKO ... #endif and other sources that are using fseeko() without testing if it's really supported (on Windows Embedded and Windows Mobile, it is not).
So, I was wondering what you think about moving these #ifdef HAVE_FSEEKO ... #endif blocks from the sources into a common include file, so that it will exists only in one place and it will be used for all other files. I can also do a patch if you want, but unfortunately it is not exactly clear to me what is the best include file to be used for this purpose.

[Carlo Bramini]

This is an example about what I'm talking about:
https://sourceforge.net/p/vice-emu/code/HEAD/tree/trunk/vice/src/core/ata.c#l53

[gpz]

I dont understand what this macro is trying to do - to me it looks like that either:

• the code is supposed to work with fseek instead of fseeko, in that case the macro should not exist and the code should use fseek insteadd
  or
• the code requires fseeko. in that case the macro should not exist either, and fseeko should be required.

what am i missing there?

[compyx]

If you want to support disks larger than 2GB -- which is common with ATA drives I think -- the code must use fseeko()/ftello(). So we must disable ATA and thus IDE64 support if there is no fseeko()/ftello(), since we cannot just write our own version of fseeko()/ftello().

[compyx]

As an alternative, there's fpos_t for stream positions, using fgetpos()/fsetpos(). But that again depends on what the fpos_t type can represent, might be a typedef for int for all we know, which then won't help either.

[gpz]

sounds like
a) this should be a configure option, else you get surprise compiling
b) that macro should indeed be at a central place and exist exactly once

[compyx]

I'd go the fpos_t route, fseek()/ftell() use long int as their offset argument, which on 64-bit Windows would still be 32-bit, so no fseek() beyond 2GB. fpos_t is the type meant for file positions since C89.

[gpz]

if that makes it work everywhere - sure. but does it? don't we exchange one set of functions we need to test for for another?

[compyx]

fgetpos()/fsetpos() are C89 functions, so no need to test. If those don't exists your compiler/libc is broken.

Problem is we use fseeko() in more than just ata.c, it's also in fsimage-probe.c, scsi.c and even util.c. So all those need updating, combined with the fact that fpos_t is an opaque type, you cannot print it or serialize it easily (for snapshots).

[compyx]

off_t util_file_length(FILE *fp) uses fseeko() without any macros, so that'll break anyway without fseeko(). And using fpos_t there cannot be done portably, so it'll need to be changed into using a stat() call.
Ugh.

[compyx]

I've moved the macros into vice.h and removed all traces of individual #defines from other files. Please check if this now works.

A proper fix for the various things I mentioned in this thread will have to wait until after the 3.6 release. So I'll keep this bug open.

[compyx]

I'm working on some archdep wrappers for fseeko()/ftello(). On Windows when they aren't available (they are using msys2, but not with MSVC 2019), I use _fseeki64()/_ftelli64().

Now the question is: are those winapi functions available for CE/Mobile? Some googling indicates they might not be.
Another thing is off_t, which appears to live in <sys/types.h> on Windows, I wonder if that is also true for CE/Mobile?

[compyx]

The replacement for util_file_length() is archdep_file_size(). The attached patch replaces all calls to util_file_length() (except in util.c itself, that'll come once I remove util_file_length().

To the other devs: please check if I made any mistakes or missed something. (this should be a pull request...)

[compyx]

Never mind, commit r41912 broke the patch.

[Carlo Bramini]

On Windows when they aren't available (they are using msys2, but not with MSVC 2019), I use _fseeki64()/_ftelli64().

Now the question is: are those winapi functions available for CE/Mobile? Some googling indicates they might not be.

Actually, no, they are not available.
My port for Windows CE / Mobile / Embedded requires an extra test like this one:

#ifdef _WIN32_WCE ... #endif

although it would be better that configure script should detect _fseeki64() and _ftelli64() functions too.
However, I noticed that it would be better to put that code only in one place for easier maintenance.

Another thing is off_t, which appears to live in <sys types.h=""> on Windows, I wonder if that is also true for CE/Mobile?</sys>

Yes, on Windows CE / Mobile / Embedded off_t exists into <sys/types.h>.
However, I have to say that there is one source that gave to me an error because off_t was undeclared since <sys/types.h> was not included. Adding the check and the inclusion of this file solved my issue. But again, there several sources with code like this:

/* Include support for 'off_t' */
#ifdef HAVE_SYS_TYPES_H
#  include <sys/types.h>
#endif

https://sourceforge.net/p/vice-emu/code/HEAD/tree/trunk/vice/src/core/ata.c#l29

So, I'm thinking: how about moving this piece of code into vice/src/types.h?
Afterall, this file is already included where the check on HAVE_SYS_TYPES_H exists.

[compyx]

I agree it's better to have configure detect _ftelli64 and _fseeki64 and finally fall back to using ftell and fseek, and indeed have src/types.h handle off_t, using a typedef for it if off_t isn't available.

That way we properly test for features instead of using assumptions about operating systems and libraries.

[compyx]

Meanwhile I've implemented using configure to detect _fseeki64/_ftelli64 and provide a fallback if even those don't exist, using old fseek/ftell. A typedef for off_t is provided, if it doesn't exist, in src/types.h.

[gpz]

what is the status of this?

[compyx]

I fixed it as well as I could. Should work in all cases and when there's no 64-bit fseek()/ftell() variant we fall back to the "old" versions. If the C library for a specific OS doesn't have 64-bit fseek what more can we do?

[gpz]

so this can be closed/fixed? :)

[compyx]

Yeah, no activity in a year, so I'll assume it now works =)
Closing.