starseeker (Mar 20 2020 at 18:48): starseeker (Mar 20 2020 at 18:48):Sean, you around to discuss the off_t issue?
Sean (Mar 20 2020 at 18:48):in a lil bit, yes, but not just this sec
starseeker (Mar 20 2020 at 18:48):k
Sean (Mar 20 2020 at 19:48):That took longer than expected! So for the off_t thing, we do already have a regression test that breaks when it's wrong I think.
Sean (Mar 20 2020 at 19:48):But the more immediate is simply compiling correctly/cleanly
starseeker (Mar 20 2020 at 19:50):If I'm understand what @Daniel Rossberg reported, we can't reliably redefine off_t as 64 bit without breaking things.
starseeker (Mar 20 2020 at 19:52):Which is not good - I counted a number of places in our own API were we expose off_t:
bu_fseek
bu_ftell
bu_vls_nibble
struct directory
struct db_i
a number of functions in rt/db_io.h
struct mem_map
struct mater
rt_color_addrec
db_dup_path_tail
starseeker (Mar 20 2020 at 19:52):If we can't safely redefine off_t some of those variables probably won't be large enough, and if we do redefine it we're breaking (at least) the fstat call.
Sean (Mar 20 2020 at 19:54):well our stuff is easy, we can make them use whatever we want. I think the issue is we were trying to not introduce a symbol if we didn't need to.
Sean (Mar 20 2020 at 19:54):but it sounds like we may need to. I don't know -- would need to see an error log or see the issue Daniel described myself.
starseeker (Mar 20 2020 at 19:54):I count seven uses of fstat in our code - if that's the only problem child we might conceivably do something at the point of those fstat calls, but now that this has happened I'm concerned that the off_t redefinition trick might start subtly breaking other things as well...
starseeker (Mar 20 2020 at 19:57):Apparently this is one of those "WONTFIX" issues for Microsoft: https://developercommunity.visualstudio.com/content/problem/308714/in-c-header-systypesh-off-t-is-defined-as-32-bit-s.html
Sean (Mar 20 2020 at 19:57):We should be popping these symbols out of config_win.h as they're encountered and detected/fixed instead of the #define trickery we're trying to get away with.
Sean (Mar 20 2020 at 19:59):probably best to introduce our own wrapper for this concept given this issue, bu_off_t
starseeker (Mar 20 2020 at 20:00):OK. I can probably rough in some of it, but I'm not really set up for proper MSVC testing at the moment.
Sean (Mar 20 2020 at 20:04):doesn't need to be complicated, probably can just test if off64_t is available, and if it is then bu_off_t is typedef'd to it, otherwise define to off_t
starseeker (Mar 20 2020 at 20:07):Right. I think for MSVC we need _off64_t as well. I remember vaguely some fun when I last tried this - a long while back I attempted to shift as much as possible of config_win.h into CMakeLists.txt and many things exploded. Fingers crossed...
starseeker (Mar 20 2020 at 20:09):Do we want to care about _FILE_OFFSET_BITS ?
Sean (Mar 20 2020 at 20:10):We don't do anything with that
starseeker (Mar 20 2020 at 20:12):It might be an issue if we don't on 32 bit systems - if we detect off64_t and go with it, but don't define _FILE_OFFSET_BITS=64, we might get some unexpected results..
Sean (Mar 20 2020 at 20:12):I think it's availability is unreliable, but would be an alternative to a cmakelist.txt test
starseeker (Mar 20 2020 at 20:12):Oh well - start small, build up as needed.
Sean (Mar 20 2020 at 20:12):Hmmm
starseeker (Mar 20 2020 at 20:13):/me goes digging for ancient config_win.h.in rework attempts...
Sean (Mar 20 2020 at 20:13):refreshing my memory on _FILE_OFFSET_BITS
starseeker (Mar 20 2020 at 20:15):I have vague memories of the stat/fstat/fseek/etc. functions having all sorts of 64 bit versions and needing to be careful to match all them up to get decent behavior, but it's been a while since I tangled with any of this... almost wonder if it's worth checking for a wrapper header on github or somewhere to see if someone's already wrapped all this...
Sean (Mar 20 2020 at 20:18):I'm certain there are solutions and we can craft something easily enough.. just will take some iterations probably.
Sean (Mar 20 2020 at 20:18):this looks like a non-windows start, for exampl:
https://github.com/the-tcpdump-group/libpcap/blob/master/cmake/Modules/FindLFS.cmake
starseeker (Mar 20 2020 at 20:22):Hmm - might be worth inquiring on the CMake list (or discourse now, I guess...)
starseeker (Mar 21 2020 at 00:31):https://github.com/zlib-ng/zlib-ng actually has a fair bit of logic related to this issue, from the looks of things...
starseeker (Mar 21 2020 at 02:16):OK, I think a defined type instead of off_t may have greatly simplified a lot of things. I had forgotten the antics needed to make the off_t trick work - I think that was actually the root cause of needing bu_fseek and bu_ftell. r75063 is a stab at switching everything over - untested on Windows.
starseeker (Mar 21 2020 at 02:31):@Daniel Rossberg I'd be curious if the latest trunk clears the issues for you - I'm not able to test the Windows code right now, so I may have missed some pieces...
Daniel Rossberg (Mar 21 2020 at 09:58):starseeker said:
Daniel Rossberg I'd be curious if the latest trunk clears the issues for you - I'm not able to test the Windows code right now, so I may have missed some pieces...
Okay, I'll test it next week.
Sean (Mar 21 2020 at 16:07):I can test windows if needed, though I'll need to install the latest msvc (I think I'm on 17)
Daniel Rossberg (Mar 22 2020 at 17:09):You could do the test with VS2017, I'll test it with VS2019 anyway.
I plan a new release of the brlcad.dll because I experienced some bugs with the current one.
Daniel Rossberg (Mar 23 2020 at 17:01):I needed to fix some compilation issues, but now the VS2019 x64 release build works. I plan to test the other builds in the next days.
starseeker (Mar 23 2020 at 17:55):Excellent!
Last updated: Oct 09 2024 at 00:44 UTC