[PATCH] ada/35953: Guard against empty buffers and end-of-connection

[Samuel Tardieu]

On 17/04, Thomas Quinot wrote:

| * Samuel Tardieu, 2008-04-17 :
| 
| > Note that if it were my choice only I would do the following two things:
| >   - optimize the TCP case to remove the useless Send/Receive in case of
| >     an empty buffer
| >   - forbid taking a stream in the UDP case, as this is most probably an
| >     error, and if it is not, there is a 99% change that the user is
| >     clueless and doesn't know he is going something which makes no sense :)
| 
| Actually it does make sense if you introduce a buffering layer over the
| stream, maybe we'll do that at some point.

I have such an implementation that I use to communicate between two
boards in a robot. But it requires an extra "Go" procedure to flush the
accumulated bytes, as there are no way to know when the packet is ready
from the user point of view.

| > I'll submit a reduced updated patch.
| 
| Thanks!

What about this one?

Note that we do not know how to distinguish between a successful 0 bytes
operation and one which caused a connection closed condition to be
encountered. I've added that in the spec.



ada/35953: Guard against empty buffers and end-of-connection

It is possible that an empty Item buffer is used with
(Item'First - Item'Last) > 1. A call to Send_Socket with such an
empty buffer will set its Last parameter ot Item'First - 1. This
would be mistakenly considered as a connection closed by peer.
This patch fixes it.

    gcc/ada/
	PR ada/35953
	* g-socket.ads (Receive_Socket, Send_Socket): Document the fact
	that using an empty buffer Item will set parameter Last to
	Item'First - 1.
	* g-socket.adb (Write): Do not mistakenly signal a socket error
	if asked to write an empty buffer as we have no way to know
	whether it is a real error or not.

Tested on i686-pc-linux-gnu and x86_64-unknown-linux-gnu with PolyORB (as
well as other submitted GNAT.Sockets patches). Ok for trunk?
---

 gcc/ada/g-socket.adb |   10 +++++-----
 gcc/ada/g-socket.ads |    4 +++-
 2 files changed, 8 insertions(+), 6 deletions(-)


diff --git a/gcc/ada/g-socket.adb b/gcc/ada/g-socket.adb
index 10ef9aa..8c96aa0 100644
--- a/gcc/ada/g-socket.adb
+++ b/gcc/ada/g-socket.adb
@@ -2113,8 +2113,8 @@ package body GNAT.Sockets is
          Last,
          Stream.To);
 
-      if Last /= Item'Last then
-         raise Socket_Error;
+      if Last < Item'Last then
+         raise Socket_Error with "packet too large for datagram";
       end if;
    end Write;
 
@@ -2129,8 +2129,8 @@ package body GNAT.Sockets is
       pragma Warnings (Off, Stream);
 
       First : Ada.Streams.Stream_Element_Offset          := Item'First;
-      Index : Ada.Streams.Stream_Element_Offset          := First - 1;
       Max   : constant Ada.Streams.Stream_Element_Offset := Item'Last;
+      Index : Ada.Streams.Stream_Element_Offset;
 
    begin
       loop
@@ -2144,8 +2144,8 @@ package body GNAT.Sockets is
          First := Index + 1;
       end loop;
 
-      if Index /= Max then
-         raise Socket_Error;
+      if Index < Max then
+         raise Socket_Error with "connection closed by peer";
       end if;
    end Write;
 
diff --git a/gcc/ada/g-socket.ads b/gcc/ada/g-socket.ads
index 55b6813..bb6cd6c 100644
--- a/gcc/ada/g-socket.ads
+++ b/gcc/ada/g-socket.ads
@@ -821,6 +821,7 @@ package GNAT.Sockets is
    --  Item'First - 1 when the socket has been closed by peer. This is not an
    --  error and no exception is raised. Flags allows to control the
    --  reception. Raise Socket_Error on error.
+   --  Last will also be set to Item'First - 1 if Item is an empty buffer.
 
    procedure Receive_Socket
      (Socket : Socket_Type;
@@ -853,9 +854,10 @@ package GNAT.Sockets is
       Last   : out Ada.Streams.Stream_Element_Offset;
       Flags  : Request_Flag_Type := No_Request_Flag);
    --  Transmit a message to another socket. Note that Last is set to
-   --  Item'First-1 when socket has been closed by peer. This is not
+   --  Item'First - 1 when socket has been closed by peer. This is not
    --  considered an error and no exception is raised. Flags allows to control
    --  the transmission. Raises Socket_Error on any other error condition.
+   --  Last will also be set to Item'First - 1 if Item is an empty buffer.
 
    procedure Send_Socket
      (Socket : Socket_Type;