Bug 249

Created attachment 195 [details]
patch

looks good to me. please, apply.

I'm not at all sure why it is a good idea to clean out all of the methods of Socket since they defined what it meant to be a socket object.  There is a reason they were there.  Can someone explain to me why did we not do the usual thing:

struct Socket
{
  virtual ~Socket();
  virtual int Bind (void) { return -1; }
};

Socket::~Socket(){}

struct UdpSocket : public Socket
{
  virtual ~UdpSocket();

  virtual int Bind (int) { return 0; }
  virtual int Bind (void) { return Socket::Bind (); }
};

UdpSocket::~UdpSocket(){}

int main (int argc, char *argv[])
{
  UdpSocket sock;
  sock.Bind (123);
  sock.Bind ();
  return 0;
}

Sorry, I already applied the patch :P

(In reply to comment #3)
> I'm not at all sure why it is a good idea to clean out all of the methods of
> Socket since they defined what it meant to be a socket object.  There is a
> reason they were there.  Can someone explain to me why did we not do the usual
> thing:
> 
> struct Socket
> {
>   virtual ~Socket();
>   virtual int Bind (void) { return -1; }
> };
> 
> Socket::~Socket(){}
> 
> struct UdpSocket : public Socket
> {
>   virtual ~UdpSocket();
> 
>   virtual int Bind (int) { return 0; }
>   virtual int Bind (void) { return Socket::Bind (); }
> };
> 
> UdpSocket::~UdpSocket(){}
> 
> int main (int argc, char *argv[])
> {
>   UdpSocket sock;
>   sock.Bind (123);
>   sock.Bind ();
>   return 0;
> }
> 

Because the virtual methods declaration is pointless?.. Do the declarations solve any problem? No.  Do they create problems? Yes!  It's very logical IMHO...

Anyway, does this solution offend you, or can I close the bug?

(In reply to comment #3)
> I'm not at all sure why it is a good idea to clean out all of the methods of
> Socket since they defined what it meant to be a socket object.  There is a
> reason they were there.  Can someone explain to me why did we not do the usual

I cannot see any reason why they were in udp-socket.h and tcp-socket.h: they just pointlessly repeated the information present in the socket.h header. There is just a single instance in our codebase where we do this in src/common/header.h and src/common/trailer.h but we do this for documentation purposes.

These classes were formerly non-abstract, but the actual implementations got pushed into *impl subclasses classes.  This fix is okay given this change.

> I cannot see any reason why they were in udp-socket.h and tcp-socket.h ...

Okay.  From flipping between several threads in several places I got the idea that we were talking about removing the declarations from *Socket.*  I guess my brain was unable to cope with an instance of a pure virtual redlaration of a pure virtual base class method in a subclass.

I agree that having 

  virtual int Bind () = 0;

in UdpSocket and TcpSocket is bizarre, causes problems and is unneccessary.

+1

changeset 266c6a334762