Kodi Community Forum

I'm looking at building xbmc on a system where fpos_t does not match the Linux implementation.

Obviously fpos_t is theoretically an opaque type and applications should not poke around inside it, but that ship has sailed in the xbmc code

emu_msvcrt.cpp is full of #if defined(_LINUX) && !defined(__APPLE__) to handle how to convert between fpos_t and off_t (and their 64bit variants), and I was trying to see if its possible to clean it up, and wanted to ask for opinions.

The core issue is that CFile exposes ->GetPosition() and ->Seek() and emu_msvcrt has to simulate fgetpos() and fsetpos() using them.

So I could replace code like:

#if defined(_LINUX) && !defined(__APPLE__)
if (dll_lseeki64(fd, *pos, SEEK_SET) >= 0)
#else
if (dll_lseeki64(fd, (__off64_t)pos->__pos, SEEK_SET) >= 0)
#endif

with something like:

#if defined(__linux__)
if (dll_lseeki64(fd, (__off64_t)pos->__pos, SEEK_SET) >= 0)
#elif defined(__foobar__)
if (dll_lseeki64(fd, (__off64_t)pos->_pos, SEEK_SET) >= 0)
#else
if (dll_lseeki64(fd, *pos, SEEK_SET) >= 0)
#endif

but wondering if a cleaner approach might be to have at the top of the file

#if defined(__linux__)
#define convert_fpos_to_off(x) (__off64_t)(x).__pos
#elif defined(__foobar__)
#define convert_fpos_to_off(x) (__off64_t)(x)._pos
#else
#define convert_fpos_to_off(x) (x)
#endif

and just use:

if (dll_lseeki64(fd, convert_fpos_to_off(*pos), SEEK_SET) >= 0)

The other question is that the dll_f{set,get}pos{,64}() only seem to be used in Win32DllLoader.cpp:win32_exports, so are they even needed in the non Win32 case?

Thanks

Win32DLLLoader:win32_exports are only used in the win32 case, yup.

Your idea sounds good to me.

(2012-03-18, 23:44)jmarshall Wrote: [ -> ]Win32DLLLoader:win32_exports are only used in the win32 case, yup.

Your idea sounds good to me.

So would you expect that a git pull request ifdeffing out the fpos_t using dll_* functions from emu_msvcrt.cpp and the fpos_t definitions from linux/PlatformDefs.h would be well received?

It's worth a go, sure - I'm not exactly the authority in this area though

I think the current trend is to use TARGET_DARWIN type stuff rather than _LINUX et. al. btw.

Cheers,
Jonathan

(2012-03-19, 00:00)jmarshall Wrote: [ -> ]It's worth a go, sure - I'm not exactly the authority in this area though

I think the current trend is to use TARGET_DARWIN type stuff rather than _LINUX et. al. btw.

Cheers,
Jonathan

OK - and if I could potentially grab another item of advice:

If I need to change cases of "only linux" which are currently expressed as

would it be better to use

or

Thanks

Looks like TARGET_LINUX is the way to go, yup.

(2012-03-19, 00:37)jmarshall Wrote: [ -> ]Looks like TARGET_LINUX is the way to go, yup.

Excellent!

"git diff|grep '^\+.*__linux__'|wc -l" shows 24 hits on my local tree, and thats only as far as JSONVariantParser in the build
time for a quick cleanup.

Thanks

TARGET_xxxx is destined to replace _LINUX, __APPLE__ and Win32 usage.

Note that TARGET_DARWIN has two sub-variants, TARGET_DARWIN_OSX and TARGET_DARWIN_IOS as there can be slight differences between OSX and iOS in some areas.

Beware that we currently also define _LINUX for OSX/iOS and that def will stay until we can migrate everything over to TARGET_xxxx usage.

Just a historical note, the reason for this is we cannot depend on the compiler providing the correct defs under all conditions and platforms and more are coming in the future.

We even have TARGET_DARWIN_IOS_ATV2 - because there are some minor differences here too which we had to take care of (just for makeing the darwin target complete hehe).