libtomath build broken on win32, due to the use of uint8_t

[nijtmans]

When importing the "develop" branch of libtom/libtommath into Tcl, it broke the win32 build. The reason is that no header file is used which defines uint8_t, still this symbol is used in tommath_private.h and bn_mp_radix_smap.c. (the use in tommath.h is harmless, since Tcl doesn't use MP_8BIT)

[nijtmans]

Suggested fix: Use "unsigned char" in stead of "uint8_t" in two places. See: http://core.tcl.tk/tcl/info/8531135e454c2231

[sjaeckel]

Can you please test if this is solved when using the msvc-friendly branch by @karel-m ? @karel-m can you please open a PR for this branch after the OK of @nijtmans ?

[sjaeckel]

@nijtmans did you have a chance to test this?

[sjaeckel]

*ping*

[nijtmans]

Well for Tcl, the msvc-friendly branch doesn't help, because uint8_t is still used. Do you want me to make a commit with my proposed change? It's orthogonal to the msvc-friendly changes. I don't have old MSVC versions to test msvc-friendly.

[czurnieden]

If I remember it correctly the proper (calling it "proper" when doing it the MS way does bear some irony, doesn't it? ;-) ) way would be

#ifdef (_MSC_VER < 1300)
     typedef unsigned char uint8_t;
#else
    typedef unsigned __int8 uint8_t;
#endif

[karel-m]

The proper way is IMO https://github.com/libtom/libtommath/blob/msvc-friendly/tommath.h#L20

But it did not help

[czurnieden]

Nothing more than the normal fixed-width integers are used in libtommath, the inclusion of the full stdint.h is not really necessary.

So, @nijtmans does it help when the lines 22 and 23 in the MSVC-friendly version of tommath.h are commented out to use the manually defined types only?

[nijtmans]

No that doesn't help, sorry, at least not for Tcl! Since Tcl doesn't want to use at all, we made some modifications to typedef the tommath types (mp_digit|mp_word) to (unsigned char|unsigned short) directly without the intermediate (uint8_t|uint16_t). So I would prefer libtommath to use either it's own types (mp_digit|mp_word) in it's implementation, either standard C types. The 2 uses of uint8_t in tommath_private.h and bn_mp_radix_smap.c could as well be replaced with "unsigned char" without any loss in functionality.

So extern const uint8_t mp_s_rmap_reverse[]; should become extern const unsigned char mp_s_rmap_reverse[]; that would make life much easier for me.

[sjaeckel]

Hmm, when I read stuff like "In the far future Tcl might be migrated to stdint types, deprecating the int/long/wide cruft." [source] I don't know what to add :) ...

... probably: "We've finally got rid of the cruft, you should now go for it as well" :) ...

I could offer

diff --git a/tommath.h b/tommath.h
index 41935d9..37682c7 100644
--- a/tommath.h
+++ b/tommath.h
@@ -19,4 +19,5 @@

-#if !defined(_MSC_VER) || _MSC_VER >= 1600
-/* supported since Microsoft Visual Studio 2010 */
+#if (!defined(_MSC_VER) || _MSC_VER >= 1600) && !defined(__THIS_ONE_DEFINE_THAT_IS_ONLY_DEFINED_ON_TCL_CORE_COMPILATION)
+/* supported since Microsoft Visual Studio 2010
+ * and TCL doesn't want stdint.h for now c.f. https://github.com/libtom/libtommath/issues/104  */
 #include <stdint.h>

but TBH you could also simply maintain something like that!?

Or is there another problem I'm not seeing?

[karel-m]

Ad libtommath + stdint.h related troubles I have a new one (HPUX) https://github.com/DCIT/perl-CryptX/issues/43

It is just FYI - you know my opinion on stdint.h, I know yours

[karel-m]

@nijtmans you can use my branch no-stdint-h (the changes: 11378f29d3) which I'll try to keep up to date with the develop branch.

But keep in mind that although I am a member of libtom projects, here I am kind of an "underground resistance group" fighting (well, currently losing) for the stdint.h-less world :)

[nijtmans]

Yes, that looks a lot more to my liking ;-) Actually, I'm not fighting, I'm just maintaining a fork of libtommath which needs to adhere to the Tcl building rules. And - actually - I'm quite satisfied with the responsiness of your team. It's difficult to satisfy everyone, I understand that. Since some people still build Tcl with MSVC 6, we still cannot depend on stdint.h, that's our fact of life. Nothing I can do about that now. Maybe in a few years or so ... when Windows Vista is dead.

[karel-m]

I know, I said "fighting", in fact I only want to make this planet a better place to live for our descendants ;)

Seriously, I am in a similar situation as I bundle libtommath + libtomcrypt with my perl module CryptX. And some guys simply build it with esoteric compilers on exotic platforms (far away from GNU linux + GNU cc + GNU make wonderland). So I see similar bugreports about "missing stdint.h" on Visual Studio pre-2010 (or even VC6 which does not support long long), HPcc compiler (for some reason only on IA64), some versions of Sun Studio compiler (which has inttypes.h but not stdint.h) etc.

[nijtmans]

Finaly, I took the time to commit the "no-stdint-h" branch as such in fossil. See: http://core.tcl.tk/tcl/timeline?r=libtommath-no-stdint.h

Tcl has a few more minor modifications, but this helps a lot. Thanks!

[karel-m]

FYI: I have just rebased no-stdint-h on the latest develop (+ pushed with --force)

[sjaeckel]

can you live with the branch maintained by @karel-m which means this could be closed, or do we have to do something else?

[nijtmans]

Yes, I can live with the current situation. You can close the ticket if you want, I don't think it's worth to keep it open.

[sjaeckel]

thx!

and thanks so much to @karel-m for maintaining these special branches!

[karel-m]

@nijtmans unfortunately I am not able to maintain no-stdint-h anymore see #309

[nijtmans]

Well, I think it's quite well possible to improve the macro trickery, making it better readable ... and restore it. I'll try to produce a pull request doing this.

[nijtmans]

I'm also not happy on how the macro's in bn_conversion look now.

[sjaeckel]

Please keep in mind that there are changes going on in #301 related to what you want to change!

[nijtmans]

Yes, I'm aware of #301. Thanks!

[czurnieden]

Yes, I'm aware of #301.

That sounds disapointed?

The single thing I did regarding bn_conversion.c is to expand the macros, polish it up a little bit and and put the result into individual files.

I took the liberty to make it a separate PR in #311 (A quick C&P from #301, hope it went well).

[karel-m]

The single thing I did regarding bn_conversion.c is to expand the macros, polish it up a little bit and and put the result into individual files.

Yes please! 👍

[minad]

I'm also not happy on how the macro's in bn_conversion look now.

@nijtmans What are you not happy about? These macros are not much different from what we had before with this SET_XLONG macros.

[nijtmans]

I'm not happy about the int##w##_t stuff, I would prefer to use the "type" as parameter. Whenever the width is needed, you can always use sizeof(type). It would have made the macro's much more readable. The SET_XLONG macro looked quite good as it was, unfortunately it was for unsigned types only.

[minad]

@nijtmans Ok, makes sense. The question is if we should keep the macros or not (see #311)

[nijtmans]

I'm OK with #311 as it is now. I'm OK with using macro's as well, but then better readable ones ;-)