Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

htonl, ntohl don't handle negative longs #44367

Closed
Rhamphoryncus mannequin opened this issue Dec 20, 2006 · 7 comments
Closed

htonl, ntohl don't handle negative longs #44367

Rhamphoryncus mannequin opened this issue Dec 20, 2006 · 7 comments
Assignees

Comments

@Rhamphoryncus
Copy link
Mannequin

Rhamphoryncus mannequin commented Dec 20, 2006

BPO 1619659
Nosy @gvanrossum, @birkenfeld

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/gvanrossum'
closed_at = <Date 2007-01-17.21:26:42.000>
created_at = <Date 2006-12-20.18:42:37.000>
labels = []
title = "htonl, ntohl don't handle negative longs"
updated_at = <Date 2007-01-17.21:26:42.000>
user = 'https://bugs.python.org/Rhamphoryncus'

bugs.python.org fields:

activity = <Date 2007-01-17.21:26:42.000>
actor = 'georg.brandl'
assignee = 'gvanrossum'
closed = True
closed_date = None
closer = None
components = ['None']
creation = <Date 2006-12-20.18:42:37.000>
creator = 'Rhamphoryncus'
dependencies = []
files = []
hgrepos = []
issue_num = 1619659
keywords = []
message_count = 7.0
messages = ['30840', '30841', '30842', '30843', '30844', '30845', '30846']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'georg.brandl', 'Rhamphoryncus', 'mark-roberts']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1619659'
versions = []

@Rhamphoryncus
Copy link
Mannequin Author

Rhamphoryncus mannequin commented Dec 20, 2006

>>> htonl(-5)
-67108865
>>> htonl(-5L)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
OverflowError: can't convert negative value to unsigned long

It works fine in 2.1 and 2.2, but fails in 2.3, 2.4, 2.5. htons, ntohs do not appear to have the bug, but I'm not 100% sure.

@Rhamphoryncus Rhamphoryncus mannequin closed this as completed Dec 20, 2006
@Rhamphoryncus Rhamphoryncus mannequin closed this as completed Dec 20, 2006
@mark-roberts
Copy link
Mannequin

mark-roberts mannequin commented Dec 26, 2006

From man page for htonl and friends:
       #include <arpa/inet.h>
       uint32_t htonl(uint32_t hostlong);
       uint16_t htons(uint16_t hostshort);
       uint32_t ntohl(uint32_t netlong);
       uint16_t ntohs(uint16_t netshort);

Python does call these underlying functions in Modules/socketmodule.c. The problem comes from that PyLong_AsUnsignedLong() called in socket_htonl() specifically checks to see that the value cannot be less than 0. The error checking was rather exquisite, I might add.

  • Mark

@Rhamphoryncus
Copy link
Mannequin Author

Rhamphoryncus mannequin commented Dec 28, 2006

I forgot to mention it, but the only reason htonl should get passed a negative number is that it (and possibly struct?) produce a negative number. Changing them to always produce positive numbers may be an alternative solution. Or we may want to do both, always producing positive while also accepting negative.

@mark-roberts
Copy link
Mannequin

mark-roberts mannequin commented Dec 30, 2006

Hmmm, yes, I see a problem. At the very least, I think we may be wanting some consistency between the acceptance of ints and longs. Also, I think we should return an unsigned long instead of just a long (which can be negative).

I've got a patch right now to make htonl, ntohl, htons, and ntohs never return a negative number. I'm rather waffling to the idea of whether we should accept negative numbers at all in any of the functions. The behavior is undefined, and it is, afterall, better not to guess what a user intended.

However, consistency should be a desirable goal, and we should accept make the interface consistent for both ints and longs.

Mark

@gvanrossum
Copy link
Member

mark-roberts, where's your patch?

@mark-roberts
Copy link
Mannequin

mark-roberts mannequin commented Jan 14, 2007

It is here:

https://sourceforge.net/tracker/index.php?func=detail&aid=1635058&group_id=5470&atid=305470

I apologize for not getting to this sooner, but I've been working like a frenzied devil at work. Things have been really hectic with our customers wanting year end reports.

@birkenfeld
Copy link
Member

Guido, you applied the patch, can this bug be closed?

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants