Skip to content

Commit

Permalink
Fix Issue #64
Browse files Browse the repository at this point in the history 
Documentation isn't clear on the matter  but it seems that the struct module has 'i' as a fixed 4 bytes and 'l' is 4 on 32bit and 8 on 64bit. Not tested on 32 bit build.
  • Loading branch information
@tonysimpson
tonysimpson authored May 3, 2018
1 parent 9417ffc commit aed1521
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion nanomsg/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class Socket(object):
"""

_INT_PACKER = _Struct(str('i'))
_INT_PACKER = _Struct(str('l'))

class _Endpoint(object):
def __init__(self, socket, endpoint_id, address):
Expand Down

4 comments on commit aed1521

@codypiersall
Copy link
Contributor

@codypiersall codypiersall commented on aed1521 Jun 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this commit is breaking things on 64-bit Linux. On Windows, 'l' and 'i' have the same size regardless of whether it's 64-bit vs 32-bit. On Linux (and MacOS) the size between the two changes.

I read the issue this was in response to (#64), but since there wasn't a test case on that issue I couldn't reproduce it. My understanding is that 'i' is the correct format code to use, because all the options in the C library really are ints -- e.g. https://github.com/nanomsg/nanomsg/blob/master/src/core/sock.c#L421 shows the switch statement that returns the right option based on the flag passed in. Based on that code, the right type to use is 'i', not 'l', at least for all those options.

@codypiersall
Copy link
Contributor

@codypiersall codypiersall commented on aed1521 Jun 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To give more information, I'm using the extension module wrapper (not ctypes), and here is some test code that fails:

import nanomsg 
s = nanomsg.Socket(nanomsg.SUB)
s.set_int_option(nanomsg.SOL_SOCKET, nanomsg.RCVMAXSIZE, -1)

The error message is

Traceback (most recent call last):                                                                                                                                            
File "test.py", line 3, in <module>                                                                                                                                           
s.set_int_option(nanomsg.SOL_SOCKET, nanomsg.RCVMAXSIZE, -1)                                                                                                              
File "/home/cody/dev/nanomsg-python/nanomsg/__init__.py", line 376, in set_int_option                                                                                         
buf))                                                                                                                                                                     
File "/home/cody/dev/nanomsg-python/nanomsg/__init__.py", line 63, in _nn_check_positive_rtn                                                                                  
raise NanoMsgAPIError()                                                                                                                                                 
nanomsg.NanoMsgAPIError: Invalid argument

@tonysimpson
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, they are all int32, apart from NN_SNDFD and NN_RCVFD which on windows are a SOCKET and 8 bits so probably just need to special case those?

@codypiersall
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!

Please sign in to comment.