Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Last Comment Bug 191187 - Request for UDP/IP (datagram) socket provider and support
: Request for UDP/IP (datagram) socket provider and support
Status: RESOLVED INCOMPLETE
:
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
-- enhancement with 7 votes (vote)
: mozilla1.9alpha5
Assigned To: Alex Fritze
:
: Patrick McManus [:mcmanus]
Mentors:
http://groups.google.com/group/netsca...
Depends on:
Blocks: 317491
  Show dependency treegraph
 
Reported: 2003-01-29 13:41 PST by Charles Melhorn
Modified: 2011-11-16 14:01 PST (History)
20 users (show)
gavin.sharp: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
dougt's patch, updated to trunk and built as part of netwerk (8.23 KB, patch)
2007-04-24 23:03 PDT, :Mook
cbiesinger: review-
Details | Diff | Splinter Review
udp socket support from zap branch (18.17 KB, patch)
2007-05-03 06:48 PDT, Alex Fritze
no flags Details | Diff | Splinter Review
now with a test (18.15 KB, patch)
2007-05-06 16:49 PDT, :Mook
cbiesinger: review+
Details | Diff | Splinter Review
udp socket provider+test, addresses comment 10 (18.00 KB, patch)
2007-05-11 16:22 PDT, :Mook
cbiesinger: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
udp socket provider+test, MPL (18.00 KB, patch)
2007-05-19 17:00 PDT, :Mook
mook.moz+mozbz: review+
mook.moz+mozbz: superreview+
Details | Diff | Splinter Review
Unbitrotting Alex patch (4.95 KB, patch)
2007-10-28 19:08 PDT, Caio Tiago Oliveira (asrail)
no flags Details | Diff | Splinter Review
Missed the new files, sorry (40.41 KB, patch)
2007-10-28 19:21 PDT, Caio Tiago Oliveira (asrail)
no flags Details | Diff | Splinter Review

Description User image Charles Melhorn 2003-01-29 13:41:10 PST
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 User image Darin Fisher 2003-01-29 14:47:10 PST
see the thread on n.p.m.netlib: http://makeashorterlink.com/?G32D32543
Comment 2 User image Alex Fritze 2005-11-22 17:31:36 PST
I have some code for this on the zap branch, but it will need some tlc before rolling it into a patch.
Comment 3 User image Henrik Skupin (:whimboo) [away 07/19 - 07/31] 2007-04-17 13:09:43 PDT
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
Comment 4 User image :Mook 2007-04-24 23:03:44 PDT
Created attachment 262730 [details] [diff] [review]
dougt's patch, updated to trunk and built as part of netwerk

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.
Comment 5 User image Christian :Biesinger (don't email me, ping me on IRC) 2007-05-01 16:31:54 PDT
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.
Comment 6 User image Alex Fritze 2007-05-03 06:47:21 PDT
(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.
Comment 7 User image Alex Fritze 2007-05-03 06:48:30 PDT
Created attachment 263590 [details] [diff] [review]
udp socket support from zap branch
Comment 8 User image :Mook 2007-05-06 16:49:46 PDT
Created attachment 263953 [details] [diff] [review]
now with a test

- 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...
Comment 9 User image Christian :Biesinger (don't email me, ping me on IRC) 2007-05-11 13:47:53 PDT
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 User image Christian :Biesinger (don't email me, ping me on IRC) 2007-05-11 14:14:56 PDT
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
Comment 11 User image :Mook 2007-05-11 16:22:16 PDT
Created attachment 264541 [details] [diff] [review]
udp socket provider+test, addresses comment 10

(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.
Comment 12 User image Boris Zbarsky [:bz] 2007-05-13 21:19:28 PDT
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).
Comment 13 User image :Mook 2007-05-19 17:00:10 PDT
Created attachment 265390 [details] [diff] [review]
udp socket provider+test, MPL

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)
Comment 14 User image Shawn Wilsher :sdwilsh 2007-05-25 18:02:05 PDT
(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.
Comment 15 User image Shawn Wilsher :sdwilsh 2007-05-25 18:05:13 PDT
Disregard previous comment, I can't read patches apparently.
Comment 16 User image timeless 2007-06-01 00:32:55 PDT
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 User image Henrik Skupin (:whimboo) [away 07/19 - 07/31] 2007-06-01 04:12:42 PDT
UDP support should be available for 1.9a5. We also have an unit test so the in-testsuite flag should be set?
Comment 18 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-06-01 07:30:52 PDT
If this has a unit test it's "in-testsuite" flag should be set to "+".
Comment 19 User image :Mook 2007-06-01 10:50:57 PDT
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 User image :Mook 2007-06-20 10:17:19 PDT
Reopening for afri's more useful part from the zap branch.
Comment 21 User image Jonathan Watt (back 21 July) [:jwatt] 2007-07-09 04:09:35 PDT
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 User image Caio Tiago Oliveira (asrail) 2007-10-28 18:43:57 PDT
Comment on attachment 263590 [details] [diff] [review]
udp socket support from zap branch

Asking for r on behalf of NSA.
Comment 23 User image Caio Tiago Oliveira (asrail) 2007-10-28 19:08:21 PDT
Created attachment 286516 [details] [diff] [review]
Unbitrotting Alex patch
Comment 24 User image Boris Zbarsky [:bz] 2007-10-28 19:11:13 PDT
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 User image Caio Tiago Oliveira (asrail) 2007-10-28 19:21:25 PDT
Created attachment 286518 [details] [diff] [review]
Missed the new files, sorry
Comment 26 User image Boris Zbarsky [:bz] 2007-10-28 19:41:21 PDT
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 User image Caio Tiago Oliveira (asrail) 2007-12-27 19:31:55 PST
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...
Comment 28 User image alexanderz 2009-01-07 14:40:18 PST
Alex, are you still planning to do this or have you abandoned it?
Comment 29 User image Alex Fritze 2009-01-07 17:39:42 PST
(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.

Note You need to log in before you can comment on or make changes to this bug.