[CLN-list] CLN and GiNaC patches for Win64 build

Jan Rheinländer jrheinlaender at gmx.de
Tue May 8 20:05:24 CEST 2018


Hello Bruno,

I placed an update to the patches on this branch:

https://github.com/jrheinlaender/cln/commits/w64

Here is what came up when I re-worked the patches as suggested by you:

> Patch 02/10 looks only partially right.
> On the other hand, the changes to sint32, uint32, sint64, uint64 look either
> broken or - in the best case - worthless and confusing.
Yes those changes were unnecessary, I removed them.

> Patch 03/10 is only partially right:
> * The cl_combine overloads should be adjusted to be consistent with patch 02/10.
#ifdef HAVE_LONGLONG
inline cl_uint cl_combine (cl_uint tag, unsigned long long value)
{ return cl_combine(tag, (cl_uint)value); }
inline cl_uint cl_combine (cl_uint tag, long long value)
{ return cl_combine(tag, (cl_uint)value); }
#endif

leads to a duplicate definition, because in types.h (after applying
patch 02/10) we have

typedef uintptr_t  uintP;
typedef uintP  cl_uint;

For the same reason, when pointer_bitsize == 64 then we need extra
overloads for the signed and unsigned long types.

>
> Patch 08/10:
> This looks wrong. I don't think you should cast an index of type 'size_t', ever.
> Can you explain the problem/issue?
The problem is that struct cl_SV in SV.h does not define any
operator[](size_t index), which is required by cl_SV_copy.cc. All
operator[] that are defined in cl_SV use the standard
operator[](unsigned long index).
Maybe this is intentional, that is, the index should not be larger than
unsigned long.
Otherwise, I suppose that struct cl_SV should be re-written to use
operator[](size_t index) as the standard operator.

> Patch 09/10:
> This patch should be dropped, once you have stopped fiddling with
> sint32, uint32, sint64, uint64 in patch 02/10.
Even then, I think this patch is required because otherwise the constructors

cln::cl_I::cl_I(long)
cln::cl_I::cl_I(unsigned long)

will not have the required

cl_I_constructor_from_L(long)
cln::cl_I_constructor_from_UL(unsigned long)

methods (linker error). In number.h, these are used on the condition
that (long_bitsize==32) but in cl_I_from_L.cc they are only included on
the condition that (cl_value_len < 32).

For Windows x64 platform, cl_value_len works out to 62:

cl_word_alignment: 4
cl_tag_len: 2
cl_value_shift: 2
pointer_bitsize: 64
intPsize:64
cl_pointer_size:
cl_value_len: 64 - 2

I changed the condition in cl_I_from_(U)L.cc to

#if (cl_value_len <= 32) || (long_bitsize == 32)

> Patch "Add missing delete operators to avoid compiler warning"
> * The 'throw' declarations are correct.
> * void operator delete (void* ptr, void* _ptr) { (void)ptr; (void)_ptr; }
>   1) This exists only since C++14. So it is a portability problem to use it.
>   2) Isn't it a memory leak if you define these methods to be a no-op?
>   Reference: http://en.cppreference.com/w/cpp/memory/new/operator_delete
Actually, the compiler tells me that since no matching delete operator
was defined, there will be a memory leak if during initialization an
exception is raised:
warning C4291: Kein zugehöriger delete-Operator gefunden; Speicher wird
nicht freigegeben, wenn die Initialisierung eine Ausnahme auslöst
But this is just a cosmetic patch anyway, I dropped it.


Jan



More information about the CLN-list mailing list