On 1/4/11, Soeren Moeller <soerenmoeller2001@gmail.com> wrote:
Ok, so it seems like at the moment substituting ULONG with sal_uIntPtr, is the safest solution, although it maybe in a next step would be a good idea to look at, where it can be replaced with a better fitting datatype. I guess that replacing ULONG with other things than sal_uIntPtr also would require correcting all callsites of the changed functions, as this supposedly would break (or at least do a cast, which possible could go wrong), while the typedef in types/solar.h at the moment means that calling a function taking sal_uIntPtr with a ULONG argument still works (and hence ULONG can be replaced one class at a time). Do I understand correctly that I without danger can replace BOOL with bool in non-virtual functions,
With the caveat about UNO api.
but should use sal_Bool or change to bool in all childs of the class at the same time for virtual functions?
today, sal_Bool = BOOL = unsigned char so changing BOOL to sal_Bool is always harmless. but then the idea - as I understand it - is to try to limit sal_Bool to UNO api related things. There is a tools developed by michael meeks to help detect virtual function mishap: see http://people.gnome.org/~michael/blog/2010-10-04.html Norbert
Sören 2011/1/4 Norbert Thiebaud <nthiebaud@gmail.com>:On 1/4/11, Soeren Moeller <soerenmoeller2001@gmail.com> wrote:Thank you for your replies, for the future I will use bool when substituting BOOL outside of the UNO API. With respect to ULONG, I, as Norbert correctly observed, used sal_uIntPtr because of the typedef in tools/solar.h, but what should I use instead? It seems there are the following possibilities: - unsigned long - sal_uInt32 (could be too short on 64-bit systems)sal_uInt32 is probably what was originally intended with most of these ULONG (back then there was no 64 bits system). the problem is that changing ULONG to sal_uInt32, like changing BOOL with bool can pose problem for virtual function. (as in: one need to change the instance of a virtual function in all the subclass at once for a given argument to avoid breaking the inheritance model)- sal_uIntPtr (wrong)sal_uIntPtr is not wrong here. it is was is _really_ behind ULONG today so that is the safest substitution, I think. Although I agree with Kohei that they look weird. When I see uintptr I always think that someone is using a dirty trick somewhere storing this into a pointer. In other words this is misleading.- C99 data types (but which of these, and as Norbert writes this would require a compat.h for Microsoft compilers, and I have no idea how to do this (and no Microsoft compiler to test it))I favor this of course, but that would require some consensus. Either we commit to use standard type or we stick with sal_xxxx, but there is no benefit in mixing the 2 forever. (note: in include magic is really not that hard, it can be stuck in solar.h to hide MS breakages) Note that would still not solve the dilemma: today ULONG is defined a uintptr_t so whether we use sal_uIntPtr or uintptr_t does not make any difference in that matter. NorbertI would like to do similar removements of dependencies on tools/ in other files in sc/inc/ (and the related source files), so it would be nice to know. which data types I should use instead. Regards Sören 2011/1/4 Norbert Thiebaud <nthiebaud@gmail.com>:On Mon, Jan 3, 2011 at 10:36 PM, Kohei Yoshida <kyoshida@novell.com> wrote:On Mon, 2011-01-03 at 21:47 +0100, Soeren Moeller wrote:Hi I have removed dependencies on tools/solar.h in some files in sc (according to http://wiki.documentfoundation.org/Easy_Hacks#write_tools.2F_pieces_out ) please review and commit.Thanks, pushed! BTW, we generally prefer the standard bool over sal_Bool, so I replaced sal_Bool with bool in your patch. The only place we need to use sal_Bool is when dealing with the UNO API. Other than that, the standard boolean type is preferred. Also, it's a bit weird to use sal_uIntPtr which isn't used much in our code base. So I replaced that with sal_uInt32.Kohei, I have not read the related code, but in principle uintptr_t and int32_t are not interchangeable. int32_t is 32 bit long, uintptr_t is supposed to be the same size than void* (that is 32 or 64 bits) in our sources, ULONG is typedef'ed as sal_uIntPrt (in tools/solar.h) , which is wrong (*) but that explain why Soeren used sal_uIntPtr. (*) it is wrong because there are multiple model of 64 bits support. Notoriously, Microsoft, as usual, instead of fixing their 64 bits support bugs, have, once again, turn their bugs into a standard and use the so-called LLP64 model, in which sizeof(long) != sizeof(void*) Note that ULONG is defined at multiple place, most of them as unsigned long (which conflict with the main definition of ULONG = sal_uIntPtr). Which raise the following question: has anyone successfully built LibreOffice for Win64 ? Note: C99 has been a standard for quite a while now. why are we not using the standardized type for these. that is: int8_t uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t, uint64_t, intptr_t, uintptr_t,... see http://en.wikipedia.org/wiki/Stdint.h Yes, I know, Microsoft still do not have a compiler compliant with the C-standard published 10 years ago... but that can be worked around with a compat.h header to hide Microsoft's screw-ups, without 'uglyfying' the rest of the code.Kohei -- Kohei Yoshida, LibreOffice hacker, Calc <kyoshida@novell.com> _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice