fpos_t handling

  Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Post Reply
abs0 Offline
Junior Member
Posts: 12
Joined: Jun 2011
Reputation: 0
Location: London
Post: #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
find quote
jmarshall Offline
Team-XBMC Developer
Posts: 26,181
Joined: Oct 2003
Reputation: 176
Post: #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: badge.gif]
find quote
abs0 Offline
Junior Member
Posts: 12
Joined: Jun 2011
Reputation: 0
Location: London
Post: #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
find quote
jmarshall Offline
Team-XBMC Developer
Posts: 26,181
Joined: Oct 2003
Reputation: 176
Post: #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: badge.gif]
find quote
abs0 Offline
Junior Member
Posts: 12
Joined: Jun 2011
Reputation: 0
Location: London
Post: #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
find quote
jmarshall Offline
Team-XBMC Developer
Posts: 26,181
Joined: Oct 2003
Reputation: 176
Post: #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: badge.gif]
find quote
abs0 Offline
Junior Member
Posts: 12
Joined: Jun 2011
Reputation: 0
Location: London
Post: #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
find quote
davilla Offline
Retired-Team-XBMC Developer
Posts: 11,479
Joined: Feb 2008
Reputation: 64
Post: #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.


MediaInfo : http://mediainfo.sourceforge.net/
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
find quote
Memphiz Offline
Team-XBMC Developer
Posts: 10,649
Joined: Feb 2011
Reputation: 112
Location: germany
Post: #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).

AppleTV2/iPhone/iPod: HowTo find debug logs and everything else which the devs like so much: click here
HowTo setup NFS for XBMC: Wiki NFS
HowTo configure avahi (zeroconf): Wiki Avahi
READ THE IOS FAQ!: iOS FAQ
find quote