[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: Ended up going with my solution as I just couldn't guarantee 100% that David's approach would always be right (for no fault of David's; at this point I just want a solution and so I'm willing to use compiler tricks to get it as I grok those). Anyway, thanks to

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Roundup Robot added the comment: New changeset 407c9c297ff7 by Brett Cannon in branch 'default': Issue #12837: Silence a Clang compiler warning on OS X. http://hg.python.org/cpython/rev/407c9c297ff7 -- ___ Python tracker

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

David Watson added the comment: On Fri 13 Dec 2013, Brett Cannon wrote: > Two things. First, I'm sorry David but my mind is not working fully enough at > the moment to see how msg_controllen is compared to cmsg_len_end without > relying on external value coming in through the parameters of the

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: Christian Heimes pointed out #ifdef __clang__ might be necessary to silence warnings on other platforms. -- ___ Python tracker ___ __

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: Two things. First, I'm sorry David but my mind is not working fully enough at the moment to see how msg_controllen is compared to cmsg_len_end without relying on external value coming in through the parameters of the function. Second, I have re-uploaded my patch

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

David Watson added the comment: Looking again at cmsg_min_space(), I see that it already returns false when msg_controllen is less than cmsg_len_end, so you could do a (signed) comparison against that, rather than 0. Patch attached. -- Added file: http://bugs.python.org/file32992/socket

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Changes by Brett Cannon : -- assignee: -> brett.cannon ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https:/

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Antoine Pitrou added the comment: On jeu., 2013-12-05 at 14:40 +, Brett Cannon wrote: > > I just don't want to perpetually be ignoring a single warning in the > code. That leads to me ignoring any warnings that come during > compilation, which is unfortunate as some warnings are actually use

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: First, sorry about the noise in the patch. Second, the patch should contain just three pragma lines: #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wtautological-compare" ... #pragma clang diagnostic pop Now I don't think that's unreada

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Charles-François Natali added the comment: I agree with Antoine. Silencing warning is fine, as long as it's not to the detriment of clarity (see Debian Openssl's vulnerability extreme example). -- ___ Python tracker

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Antoine Pitrou added the comment: Is there any point in littering the source code to avoid platform-specific warnings? The difference is probably that some fields are defined unsigned on OS X while they're signed on other platforms. (also, your patch has lots of unrelated changes) --

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Christian Heimes added the comment: I'd like to see the warning silenced before 3.4 gets released, too. How about a check like (Py_ssize_t)msg->msg_controllen > 0xL instead? I'd also be fine with pragmas. -- nosy: +christian.heimes versions: +Python 3.4 ___

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: Attached is a patch that silences the warning for just the 'if' statement under Clang using pragmas. I don't know if we have ever had it out on python-dev on our view of using pragmas, but this seems like a reasonable place to use it. -- Added file: http

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: This is still a warning and so I'm still looking for a solution. -- ___ Python tracker ___ ___ Python-

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: Since we never solved this I'm still getting compiler warnings. Can we decide on a solution for this so I can go back to be warning-free (sans `-Wno-unused-value -Wno-empty-body -Qunused-arguments`)? /Users/bcannon/Developer/repo/cpython/bootstrap_importlib/Mo

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Antoine Pitrou added the comment: If you're casting to a larger signed type, then the semantics change, since there is a sign extension. For example (unsigned int) 0x could be cast to (long long) -1. You could cast to size_t instead and compare the result to SOCKLEN_T_MAX (which curre

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Nick Coghlan added the comment: I tend to be fairly paranoid about operating systems doing occasionally bizarre things, so I like having clearly defined behaviour even when the OS does something arguably nuts, but permitted by the relevant standard. Hence I think keeping the check is the righ

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Charles-François Natali added the comment: `long long` is not ANSI, but C99. Anyhow, I'm still not sure this check is necessary, because: 1) I really doubt any modern OS uses a signed socklen_t 2) even if it's the case, I don't see how we could possibly end up with a negative msg_controllen I'

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Brett Cannon added the comment: This is now generating a compiler warning under OS X because the older POSIX standard is followed where socklen_t can be unsigned. Attached is a patch to cast msg_controllen to a size big enough to hold either signed 2**31-1 or unsigned 2**32-1 (i.e., long long

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Charles-François Natali added the comment: Thanks for the patch. For the record, here's Linus Torvalds' opinion on this whole socklen_t confusion: """ _Any_ sane library _must_ have "socklen_t" be the same size as int. Anything else breaks any BSD socket layer stuff. POSIX initially did mak

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Roundup Robot added the comment: New changeset 3ed2d087e70d by Charles-François Natali in branch 'default': Issue #12837: POSIX.1-2008 allows socklen_t to be a signed integer: re-enable http://hg.python.org/cpython/rev/3ed2d087e70d -- nosy: +python-dev _

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Charles-François Natali added the comment: > That has since been changed. I'm reading from POSIX.1-2008, > which says: I see. > The warning against using values larger than 2**32 - 1 is still > there, I presume because they would not fit in a 32-bit signed > int. I assume you mean 2**31 - 1.

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

David Watson added the comment: On Wed 24 Aug 2011, Charles-François Natali wrote: > > I included this test deliberately, because msg_controllen may be > > of signed type [...] POSIX allows socklen_t to be signed > > http://pubs.opengroup.org/onlinepubs/007908799/xns/syssocket.h.html > """ >

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Charles-François Natali added the comment: I checked in glibc, FreeBSD and OpenBSD source codes, and they all define socklen_t as an unsigned integer. I think the confusion arises from this: """ The third argument of accept() was originally declared as an int * (and is that under libc4

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Charles-François Natali added the comment: > I included this test deliberately, because msg_controllen may be > of signed type [...] POSIX allows socklen_t to be signed http://pubs.opengroup.org/onlinepubs/007908799/xns/syssocket.h.html """ makes available a type, socklen_t, which is an unsig

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

Changes by Antoine Pitrou : -- nosy: +neologix ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pyth

[issue12837] Patch for issue #12810 removed a valid check on socket ancillary data

New submission from David Watson : Changeset 4736e172fa61 for issue #12810 removed the test "msg->msg_controllen < 0" from socketmodule.c, where msg_controllen happened to be unsigned on the reporter's system. I included this test deliberately, because msg_controllen may be of signed type (