packet_* => tstream glue

[Stefan (metze) Metzmacher]

Hi Andrew,

here's the wrapper to use the packet_* full_request functions with tstream.

http://gitweb.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=7123249cbbc8c5b72d1fbe4936cbd2b6cd4ddee9

(I haven't compiled it yet...)

metze

[Andrew Bartlett]

On Wed, 2009-11-04 at 16:20 +0100, Stefan (metze) Metzmacher wrote:
> Hi Andrew,
>
> here's the wrapper to use the packet_* full_request functions with tstream.
>
> http://gitweb.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=7123249cbbc8c5b72d1fbe4936cbd2b6cd4ddee9
>
> (I haven't compiled it yet...)

And I've not tried it, but it does look like it could be very useful.

The use of 'send' and 'recv' in the tsocket code is really quite
confusing.  In this case, it means 'start' and 'conclude' (or 'finish')
the asynchronous operations, rather than to send() and recv() packets.

Would it be at all possible to change those names?  It really bends the
mind a bit too much.

Thanks,

Andrew Bartlett

--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.

[Volker Lendecke]

On Thu, Nov 05, 2009 at 08:39:43AM +1100, Andrew Bartlett wrote:
> The use of 'send' and 'recv' in the tsocket code is really quite
> confusing.  In this case, it means 'start' and 'conclude' (or 'finish')
> the asynchronous operations, rather than to send() and recv() packets.
>
> Would it be at all possible to change those names?  It really bends the
> mind a bit too much.

Please don't. tsocket_recv_send and tsocket_recv_recv is
indeed confusing, but all the other async functions use
_send and _recv. With the async functions you usually don't
use these low-level direct async counterparts of the sendto
and recvfrom syscalls much anyway, in Samba3 we mostly use
the writev and read_packet which are much more powerful.

Volker

[Andrew Bartlett]

On Wed, 2009-11-04 at 23:07 +0100, Volker Lendecke wrote:

> On Thu, Nov 05, 2009 at 08:39:43AM +1100, Andrew Bartlett wrote:
> > The use of 'send' and 'recv' in the tsocket code is really quite
> > confusing.  In this case, it means 'start' and 'conclude' (or 'finish')
> > the asynchronous operations, rather than to send() and recv() packets.
> >
> > Would it be at all possible to change those names?  It really bends the
> > mind a bit too much.
>
> Please don't. tsocket_recv_send and tsocket_recv_recv is
> indeed confusing, but all the other async functions use
> _send and _recv.

Which I think shows that, while it makes sense for 'send and recv' a
packet, that is is a really poor choice of naming pattern for a general
infrastructure.

> With the async functions you usually don't
> use these low-level direct async counterparts of the sendto
> and recvfrom syscalls much anyway, in Samba3 we mostly use
> the writev and read_packet which are much more powerful.

I still disagree that the pattern justifies such a ridiculous name.  Do
you have any other suggestions?  I really think what we have now is
impossible to explain to others.

Thanks,

Andrew Bartlett

--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.

[Volker Lendecke]

On Thu, Nov 05, 2009 at 09:25:47AM +1100, Andrew Bartlett wrote:

> On Wed, 2009-11-04 at 23:07 +0100, Volker Lendecke wrote:
> > On Thu, Nov 05, 2009 at 08:39:43AM +1100, Andrew Bartlett wrote:
> > > The use of 'send' and 'recv' in the tsocket code is really quite
> > > confusing.  In this case, it means 'start' and 'conclude' (or 'finish')
> > > the asynchronous operations, rather than to send() and recv() packets.
> > >
> > > Would it be at all possible to change those names?  It really bends the
> > > mind a bit too much.
> >
> > Please don't. tsocket_recv_send and tsocket_recv_recv is
> > indeed confusing, but all the other async functions use
> > _send and _recv.
>
> Which I think shows that, while it makes sense for 'send and recv' a
> packet, that is is a really poor choice of naming pattern for a general
> infrastructure.
>
> > With the async functions you usually don't
> > use these low-level direct async counterparts of the sendto
> > and recvfrom syscalls much anyway, in Samba3 we mostly use
> > the writev and read_packet which are much more powerful.
>
> I still disagree that the pattern justifies such a ridiculous name.  Do
> you have any other suggestions?  I really think what we have now is
> impossible to explain to others.

vlendec@sam[SerNet]:~/git/s3-work/source4 > git grep _send\( | wc -l
1374

Enjoy re-writing them all in Samba4 first :-))

Volker

[Andrew Bartlett]

On Wed, 2009-11-04 at 23:34 +0100, Volker Lendecke wrote:

> On Thu, Nov 05, 2009 at 09:25:47AM +1100, Andrew Bartlett wrote:
> > On Wed, 2009-11-04 at 23:07 +0100, Volker Lendecke wrote:
> > > On Thu, Nov 05, 2009 at 08:39:43AM +1100, Andrew Bartlett wrote:
> > > > The use of 'send' and 'recv' in the tsocket code is really quite
> > > > confusing.  In this case, it means 'start' and 'conclude' (or 'finish')
> > > > the asynchronous operations, rather than to send() and recv() packets.
> > > >
> > > > Would it be at all possible to change those names?  It really bends the
> > > > mind a bit too much.
> > >
> > > Please don't. tsocket_recv_send and tsocket_recv_recv is
> > > indeed confusing, but all the other async functions use
> > > _send and _recv.
> >
> > Which I think shows that, while it makes sense for 'send and recv' a
> > packet, that is is a really poor choice of naming pattern for a general
> > infrastructure.
> >
> > > With the async functions you usually don't
> > > use these low-level direct async counterparts of the sendto
> > > and recvfrom syscalls much anyway, in Samba3 we mostly use
> > > the writev and read_packet which are much more powerful.
> >
> > I still disagree that the pattern justifies such a ridiculous name.  Do
> > you have any other suggestions?  I really think what we have now is
> > impossible to explain to others.
>
> vlendec@sam[SerNet]:~/git/s3-work/source4 > git grep _send\( | wc -l
> 1374
>
> Enjoy re-writing them all in Samba4 first :-))

Sure - as I said, where they represent the send() and recv() of packets,
it makes a lot of sense.  But where there is no longer a resonable
mapping to the concept of send() and recv() as understood in unix
systems programming then the name is no longer a good choice.

There may be parts of Samba4 that indeed do need such a rename as well.

Andrew Bartlett

--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.

[simo-7]

On Thu, 2009-11-05 at 09:25 +1100, Andrew Bartlett wrote:

> On Wed, 2009-11-04 at 23:07 +0100, Volker Lendecke wrote:
> > On Thu, Nov 05, 2009 at 08:39:43AM +1100, Andrew Bartlett wrote:
> > > The use of 'send' and 'recv' in the tsocket code is really quite
> > > confusing.  In this case, it means 'start' and 'conclude' (or 'finish')
> > > the asynchronous operations, rather than to send() and recv() packets.
> > >
> > > Would it be at all possible to change those names?  It really bends the
> > > mind a bit too much.
> >
> > Please don't. tsocket_recv_send and tsocket_recv_recv is
> > indeed confusing, but all the other async functions use
> > _send and _recv.
>
> Which I think shows that, while it makes sense for 'send and recv' a
> packet, that is is a really poor choice of naming pattern for a general
> infrastructure.
>
> > With the async functions you usually don't
> > use these low-level direct async counterparts of the sendto
> > and recvfrom syscalls much anyway, in Samba3 we mostly use
> > the writev and read_packet which are much more powerful.
>
> I still disagree that the pattern justifies such a ridiculous name.  Do
> you have any other suggestions?  I really think what we have now is
> impossible to explain to others.

What's hard to explain in a convention ?
A single bad case is not enough to justify changing a lot of code and a
solid convention through all our code.

Put there a comment if you really need to.

Simo.

--
Simo Sorce
Samba Team GPL Compliance Officer <simo@...>
Principal Software Engineer at Red Hat, Inc. <simo@...>

[Volker Lendecke]

On Thu, Nov 05, 2009 at 09:47:53AM +1100, Andrew Bartlett wrote:
> Sure - as I said, where they represent the send() and recv() of packets,
> it makes a lot of sense.  But where there is no longer a resonable
> mapping to the concept of send() and recv() as understood in unix
> systems programming then the name is no longer a good choice.

There are tons of functions in Samba4 as well where the
_send/_recv pair represents a whole set of computations
(both sending and receiving stuff) done asynchonously. Just
look at all the composite functions that are the basis for
our current tevent_req stuff.

This convention is around now for many years and has really
sunk in. It's a bit late to change now, sorry.

Volker

[Stefan (metze) Metzmacher]

Andrew Bartlett schrieb:

> On Wed, 2009-11-04 at 16:20 +0100, Stefan (metze) Metzmacher wrote:
>> Hi Andrew,
>>
>> here's the wrapper to use the packet_* full_request functions with tstream.
>>
>> http://gitweb.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=7123249cbbc8c5b72d1fbe4936cbd2b6cd4ddee9
>>
>> (I haven't compiled it yet...)
>
> And I've not tried it, but it does look like it could be very useful.

I have some local changes to it, I hope to get it ready to push within
this week.

> The use of 'send' and 'recv' in the tsocket code is really quite
> confusing.

that's why I don't added tstream_recv_recv and tstream_send_send,
but tstream_readv_recv and tstream_writev_send.

> In this case, it means 'start' and 'conclude' (or 'finish')
> the asynchronous operations, rather than to send() and recv() packets.
>
> Would it be at all possible to change those names?  It really bends the
> mind a bit too much.

The caller sends its desire to get a result of a specific task with the
_send function. And gets the result with the _recv function.

For me it's not confusing, but I'm also not a native speaker...

metze