fpos_t handling
#1
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 Smile

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 Smile
Reply
#2
Win32DLLLoader:win32_exports are only used in the win32 case, yup.

Your idea sounds good to me.
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#3
(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? Smile
Reply
#4
It's worth a go, sure - I'm not exactly the authority in this area though Wink

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

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#5
(2012-03-19, 00:00)jmarshall Wrote: It's worth a go, sure - I'm not exactly the authority in this area though Wink

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
Code:
#if defined (_LINUX) && !defined(__APPLE__)

would it be better to use
Code:
#if defined(__linux__)

or
Code:
#if defined(TARGET_LINUX)

Thanks Smile
Reply
#6
Looks like TARGET_LINUX is the way to go, yup.
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#7
(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
Reply
#8
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.
Reply
#9
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).
AppleTV4/iPhone/iPod/iPad: HowTo find debug logs and everything else which the devs like so much: click here
HowTo setup NFS for Kodi: NFS (wiki)
HowTo configure avahi (zeroconf): Avahi_Zeroconf (wiki)
READ THE IOS FAQ!: iOS FAQ (wiki)
Reply

Logout Mark Read Team Forum Stats Members Help
fpos_t handling0