Removed bn_conversion.c in favor of individual files

[czurnieden]

In reaction to #104 and #309.
It also makes development in #301 easier.

[nijtmans]

I'm OK now (the type-cast / negation order is not crucial, I even think that other compilers might disagree with MSVC ....)

[czurnieden]

I'm OK now (the type-cast / negation order is not crucial, I even think that other compilers might disagree with MSVC ....)

They way it is done makes it a defined behaviour but does not solve the problem with overflow per se. Put a note in the documentation or is this behaviour obvious?

[minad]

They way it is done makes it a defined behaviour but does not solve the problem with overflow per se.

No. The way negation is done now solves the undefined behavior, since overflow of unsigned numbers is well defined.

[minad]

Hmm, the macros were quite ugly and generic programming in c simply sucks. However now we also get a bit of code duplication and the functions can get out of sync, which I am not very fond of. #109 and #309 are non-issues and #301 can also be solved with bn_conversion.c as is.

[minad]

@czurnieden Before we had these MP_SET_XLONG macros, bn_conversion does the same. Keeping the macros might not make sense, all the macros are used only twice. If you want to fix #301, you could simply split bn_conversion in multiple files, keeping the macros. However we could think of supporting multiple functions per file as introduced in bn_deprecated.c and bn_conversion.c. This was in fact not my idea, but @sjaeckel suggested to put the deprecated stuff in a single file. But I think it is a good idea actually. For the bn_conversion case, splitting might make sense.

[czurnieden]

No. The way negation is done now solves the undefined behavior, since overflow of unsigned numbers is well defined.

Ah, misunderstanding.
I meant the actual overflow, that is if mp_get_magx() returns a value larger than 2^(x-1). The behaviour of the code is defined but the user does not have an easy way to detect it. Normally a flag would be set like e.g.: errno but we don't use such flags (It would be a real pain in the behind to implement it). The only way to be sure is to check the size of the input which has to be done by the caller, so I'd like to propose a little reminder in the documentation, maybe something like: "The size of the input must be checked by the caller."

However we could think of supporting multiple functions per file as introduced in bn_deprecated.c and bn_conversion.c

The stuff in bn_deprecated.c is, well, deprecated, so it does not make a lot of sense to invest in them.

bn_conversion.c on the other hand has all the new stuff in it. It is helpful for at least two people here if it gets cut in multiple files.

split bn_conversion in multiple files, keeping the macros

The macros are not needed, they do not rely on many compile-time variables, they are just generating code. This PR keeps the necessary compile-time variables (constants actually, like e.g.: MP_DIGIT_BIT) and cuts it in individual files.

It is just a different formatting, no change in functionality.

However now we also get a bit of code duplication

Yes. And?
The macros would have generated the same code with the same duplications.

and the functions can get out of sync,

Out of sync with what? bn_conversion.c? That file doesn't exist anymore. With their "twins"? Yes, that can happen, so a bit more care is necessary when repairing one of the functions, you have to check their "twin", too.

With bn_conversion.c cut into individual files it now follows the LTM principle of "one function per file" and it's naming convention. With the exceptions bn_prime_tab.c, bn_cutoffs.c containing constants and of course bn_deprecated.c the single exception with several functions in it and a different naming scheme.

[minad]

With their "twins"? Yes, that can happen, so a bit more care is necessary when repairing one of the functions, you have to check their "twin", too.

@czurnieden Do you think it makes sense to keep the macros to avoid this out of sync issue between the twins? The macros should be improved to be more readable however and get a type argument instead of a bit integer argument as @nijtmans suggested.

[czurnieden]

@minad f'up at #313?

[minad]

@czurnieden Ok to close this in favor of #313?