Closed
Bug 191187
Opened 22 years ago
Closed 16 years ago
Request for UDP/IP (datagram) socket provider and support
Categories
(Core :: Networking, enhancement)
Core
Networking
Tracking
()
RESOLVED
INCOMPLETE
mozilla1.9alpha5
People
(Reporter: cmelhorn, Assigned: alex)
References
()
Details
Attachments
(2 files, 5 obsolete files)
18.00 KB,
patch
|
Mook
:
review+
Mook
:
superreview+
|
Details | Diff | Splinter Review |
40.41 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2.1) Gecko/20021130 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2.1) Gecko/20021130 Currently, the Netlib CreateTransportOfType() method on nsISocketTransportService provides only the ability to create TCP/IP sockets or tcp variants: tcp+SSL, tcp+TLS, tcp+SOCKS, and tcp+SOCKS4. It would be useful if you could extend the library's set of socket providers to include one for UDP (datagram) sockets, perhaps by making use of the portable runtime function PR_NewUDPSocket() and related routines, and provide general support for (scriptable) UDP sockets as necessary. Reproducible: Always Steps to Reproduce: 1. 2. 3.
Comment 1•22 years ago
|
||
see the thread on n.p.m.netlib: http://makeashorterlink.com/?G32D32543
Assignee | ||
Comment 2•19 years ago
|
||
I have some code for this on the zap branch, but it will need some tlc before rolling it into a patch.
Blocks: 317491
Updated•18 years ago
|
Comment 3•18 years ago
|
||
Alex, do you have any update for us what's happen with your code? Is it in a state to create a patch? It would be very helpful for me due to I need to send UDP datagrams. Or do you have another hint? Thanks
Updated•18 years ago
|
Assignee: dougt → nobody
QA Contact: benc → networking
This is just dougt's patch (from the newsgroups) made to compile on trunk and built in netwerk. It seems to be good enough to send something and get a response, at least.
Assignee: nobody → mook.moz+mozbz
Status: NEW → ASSIGNED
Attachment #262730 -
Flags: review?(cbiesinger)
Comment 5•18 years ago
|
||
Comment on attachment 262730 [details] [diff] [review] dougt's patch, updated to trunk and built as part of netwerk - please use consistent indentation in netwerk/socket/base/nsUDPSocketProvider.cpp, ideally 4-space per the style in this directory (same for the .h) - where did you copy the code in that file from, given that it was supposedly created by Netscape in 1998? (same for the .h) - AddToSocket should get an NS_NOTREACHED and a failure return (NS_ERROR_UNEXPECTED, I guess) this needs a unit test before this is checked in.
Attachment #262730 -
Flags: review?(cbiesinger) → review-
Assignee | ||
Comment 6•18 years ago
|
||
(In reply to comment #3) > Alex, do you have any update for us what's happen with your code? Is it in a > state to create a patch? It would be very helpful for me due to I need to send > UDP datagrams. Or do you have another hint? Thanks > Henrik: Sorry for the lag - I'm gonna attach a patch with what I have in a minute. It's basic, but works well for me in zap.
Assignee | ||
Comment 7•18 years ago
|
||
- indentation fixed - the 1998 files are from dougt's newsgroup post; I left the copyrights as-is because as far as I know that's it. (The new test, which I did write from scratch, has (c) 2007) - AddToSocket fixed. Of course, now that Alex attached his patch, I have no idea where to go from here - his patch actually makes more sense (in that it has UDP-specific things). This one is still good enough to send something and receive responses, but that's about it...
Attachment #262730 -
Attachment is obsolete: true
Attachment #263953 -
Flags: review?(cbiesinger)
Comment 9•18 years ago
|
||
OK, so I do want to take something like afri's patch, since that allows listening for datagrams too. License headers would be good though; and I didn't do a real review of the interface (or implementation, for that matter) yet. It also makes it easier to tell where the boundaries of the individual datagrams are. Now... Mook's patch is nice in that it parallels TCP. It now occurred to me though that it has the problem that you don't see packet boundaries there, which can be a problem because datagrams can get lost... hm...
Comment 10•18 years ago
|
||
Comment on attachment 263953 [details] [diff] [review] now with a test OK, this approach is probably good enough too. I still want to take something like afri's patch in addition. seems like you could use PR_ErrorToString instead of the buffer allocation stuff + rv = outstream->Write((char*)&data, sizeof(PRUint32), &count); please don't cast away const, that's not nice
Attachment #263953 -
Flags: review?(cbiesinger) → review+
Comment 11•18 years ago
|
||
(In reply to comment #10) > > seems like you could use PR_ErrorToString instead of the buffer allocation > stuff > Fixed, didn't know that existed :) > + rv = outstream->Write((char*)&data, sizeof(PRUint32), &count); > > please don't cast away const, that's not nice > Fixed, that was silly of me.
Attachment #263953 -
Attachment is obsolete: true
Attachment #264541 -
Flags: review?(cbiesinger)
Updated•18 years ago
|
Attachment #264541 -
Flags: review?(cbiesinger) → review+
Attachment #264541 -
Flags: superreview?(bzbarsky)
Comment 12•18 years ago
|
||
Comment on attachment 264541 [details] [diff] [review] udp socket provider+test, addresses comment 10 >Index: netwerk/socket/base/nsUDPSocketProvider.cpp >+ * Version: NPL 1.1/GPL 2.0/LGPL 2.1 Really? And is Netscape really the original developer? Check with dougt on the latter; and use MPL when landing this, no matter what the answer to that question is. There should be no more NPL landing into the tree. Same thing for the header. With that, sr=bzbarsky (note: I didn't really read the test code).
Attachment #264541 -
Flags: superreview?(bzbarsky) → superreview+
Comment 13•18 years ago
|
||
Updated to MPL, sorry about that. Carrying forward r/sr I did ask dougt on IRC, and as far as I understand his response, yes, Netscape/1998. (Sorry about the delay, my machine was having fun deciding that it no can no longer link libxul for some reason... bah)
Attachment #264541 -
Attachment is obsolete: true
Attachment #265390 -
Flags: superreview+
Attachment #265390 -
Flags: review+
Updated•18 years ago
|
Keywords: helpwanted
Whiteboard: [checkin needed]
Comment 14•18 years ago
|
||
(In reply to comment #5) > this needs a unit test before this is checked in. Ahem. Please get this, then I can check it in.
Whiteboard: [checkin needed]
Comment 15•18 years ago
|
||
Disregard previous comment, I can't read patches apparently.
Whiteboard: [checkin needed]
Comment 16•17 years ago
|
||
Comment on attachment 265390 [details] [diff] [review] udp socket provider+test, MPL mozilla/netwerk/build/nsNetCID.h 1.66 mozilla/netwerk/build/nsNetModule.cpp 1.141 mozilla/netwerk/socket/base/Makefile.in 1.22 mozilla/netwerk/socket/base/nsUDPSocketProvider.cpp 1.1 mozilla/netwerk/socket/base/nsUDPSocketProvider.h 1.1 mozilla/netwerk/test/Makefile.in 1.95 mozilla/netwerk/test/TestUDPSocketProvider.cpp 1.1
Comment 17•17 years ago
|
||
UDP support should be available for 1.9a5. We also have an unit test so the in-testsuite flag should be set?
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: Future → mozilla1.9alpha5
Updated•17 years ago
|
Flags: in-testsuite?
Comment 18•17 years ago
|
||
If this has a unit test it's "in-testsuite" flag should be set to "+".
Flags: in-testsuite? → in-testsuite+
Comment 19•17 years ago
|
||
Umm, there's still afri's zap branch thing that provides UDP-specific support... Should that be spun into a new bug or something? Or reopen this one? Because the socket provider itself doesn't really expose everything UDP would make sense for...
Comment 20•17 years ago
|
||
Reopening for afri's more useful part from the zap branch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 21•17 years ago
|
||
Alex has been way too busy to pay attention to bugzilla for some time, but he's more than happy for people to pick up any of his stuff and run with it (as Robert did in bug 238324). Feel free to work on anything you find useful from the zap to get it landed on trunk.
Comment 22•17 years ago
|
||
Comment on attachment 263590 [details] [diff] [review] udp socket support from zap branch Asking for r on behalf of NSA.
Attachment #263590 -
Flags: review?(bzbarsky)
Comment 23•17 years ago
|
||
Attachment #263590 -
Attachment is obsolete: true
Attachment #286516 -
Flags: review?(bzbarsky)
Attachment #263590 -
Flags: review?(bzbarsky)
Comment 24•17 years ago
|
||
1) I doubt I can do serious reviews here until sometime in the spring. 2) This last patch is clearly missing pieces (e.g. nsUDPSocket.cpp).
Comment 25•17 years ago
|
||
Attachment #286516 -
Attachment is obsolete: true
Attachment #286518 -
Flags: review?(bzbarsky)
Attachment #286516 -
Flags: review?(bzbarsky)
Comment 26•17 years ago
|
||
See comment 24 item 1. I don't really know our socket code well, so I'll have to learn it to review this. You might have better luck with biesi.
Comment 27•17 years ago
|
||
Comment on attachment 286518 [details] [diff] [review] Missed the new files, sorry Asking for biesi review. I was not on the CC list and haven't seen bz comments...
Attachment #286518 -
Flags: review?(bzbarsky) → review?(cbiesinger)
Comment 28•16 years ago
|
||
Alex, are you still planning to do this or have you abandoned it?
Assignee | ||
Comment 29•16 years ago
|
||
(In reply to comment #28) > Alex, are you still planning to do this or have you abandoned it? The patch is pretty much obsolete. There is much more complete UDP socket support in the zap repository (http://hg.mozilla.org/users/alex_croczilla.com/zap-central/file/ecd584fc9599/zap/netutils/src/). I might try to port this to Mozilla proper at some point in the future, but that should really be a bug on its own. So I'm closing this one.
Status: NEW → RESOLVED
Closed: 17 years ago → 16 years ago
Resolution: --- → INCOMPLETE
Updated•13 years ago
|
Attachment #286518 -
Flags: review?(cbiesinger)
You need to log in
before you can comment on or make changes to this bug.
Description
•