Alexander
Alexander

Reputation: 235

double to PyFloat conversion is incorrect

I'm learning SWIG, for using C in Python. I've written this function, but I can't understand, why the wrapped myfunc returns wrong float/double values:

mfuncs.c

#include <stdlib.h>

float myfunc(int n) {
    float result;
    result = 100 / n;
    return result;
}

mfuncs.i

%module mfuncs
%typemap(out) double, float "$result = PyFloat_FromDouble($1);"

extern float myfunc(int n);

Finally I get 1107558400.0 instead of 33.33333.

>>> import mfuncs
>>> mfuncs.myfunc(3)
1107558400.0
>>> 

Where is the mistake?

Upvotes: 2

Views: 636

Answers (2)

Adam
Adam

Reputation: 17369

You have two bugs in this code that prevent myfunc(3) from returning that 33.3333 you expect. Flexo wonderfully explained the wrapping problem. The other problem is this line

    result = 100 / n;

100 is an int, n is an int, the result of the division will be an int, and THEN cast to float. So myfunc(3) == 33. Python 2 acts the same way, it has been changed in Python 3 such that the division of two ints results in a float.

Just change that line to

    result = 100.0 / n;

or make n a float/double.

Upvotes: 1

The SWIG typemap is unneeded - it's provided by default, you only need to write typemaps for "esoteric" types and the default double/float ones provided are fine here.

The real problem here is that you're not compiling with warnings enabled or ignoring them! It's really worth getting in the habit of compiling with "-Wall -Wextra" or whatever your compiler requires to enable maximum warnings and heeding them.

Your SWIG interface only tells SWIG about the function myfunc but there's nothing in that interface to make the declaration available to the compiler you use to compile the generated myfuncs_wrap.c. This means that when you come to compile the shared library you're relying on an implicit declaration of myfunc. GCC on my machine with -Wall reports this:

test_wrap.c:3139:3: warning: implicit declaration of function 'myfunc'

The implicit declaration assumes it returns int. That's just the rule in C if there is no declaration, it's as though you wrote:

#include <stdlib.h>

int myfunc(int n);

int main() {
  printf("%d\n", myfunc(3));   
  return 0;
}

which is clearly wrong (undefined behaviour to be exact) given the definition of myfunc returns a float. Your implementation (legally) chooses to do the simplest thing for this undefined behaviour, which is roughly a bit-wise cast from int to float. (It could equally well do anything, even something different on every run - that's the beauty of undefined behaviour).

You can fix your SWIG interface by changing it to:

%module mfuncs
%{
extern float myfunc(int n);
%}

extern float myfunc(int n);

this works because the code between %{ and %} is directly passed to the generated wrapper, which makes the compiler aware of the real declaration of myfunc when building the wrapper.

There's a nicer solution in my view though: provide the declaration only once, in a header file and then your interface file becomes:

%module mfuncs
%{
#include "myfunc.h"
%}

%include "myfunc.h"

(and obviously #include "myfunc.h" in myfunc.c). In that way you only write the declaration once and the compiler will warn/error if there's anything that's not quite expected rather than just take a (usually wrong) best guess.

Upvotes: 4

Related Questions