+- Kodi Community Forum (https://forum.kodi.tv)
+-- Forum: Development (https://forum.kodi.tv/forumdisplay.php?fid=32)
+--- Forum: Kodi Application (https://forum.kodi.tv/forumdisplay.php?fid=93)
+--- Thread: fpos_t handling (/showthread.php?tid=125928)
fpos_t handling - abs0 - 2012-03-18
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
RE: fpos_t handling - jmarshall - 2012-03-18
Win32DLLLoader:win32_exports are only used in the win32 case, yup.
Your idea sounds good to me.
RE: fpos_t handling - abs0 - 2012-03-18
(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?
RE: fpos_t handling - jmarshall - 2012-03-19
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
RE: fpos_t handling - abs0 - 2012-03-19
(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
Code:
#if defined (_LINUX) && !defined(__APPLE__)would it be better to use
Code:
#if defined(__linux__)or
Code:
#if defined(TARGET_LINUX)Thanks
RE: fpos_t handling - jmarshall - 2012-03-19
Looks like TARGET_LINUX is the way to go, yup.
RE: fpos_t handling - abs0 - 2012-03-19
(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
RE: fpos_t handling - davilla - 2012-03-19
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.
RE: fpos_t handling - Memphiz - 2012-03-19
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).