Problem by subclassing the QIODevice

[wysota]

OK, so can you say to me how can I replace my QTcpServer by a QNetworkReply ? The QtAssistant doesn't illustrate this case...
For now, I'm using the QTcpServer::listen function for waiting client connexions, and creating a QTcpSocket to communicate with this client.

Come on. Read my posts carefully, I never said you should replace QTcpServer with QNetworkReply.

[erqsor]

Ok, misunderstanding, sorry...

No. I thought you were interested in the client side because QAbstractSocket represents a client side of the connection. The server is implemented by Q*Server.

But I need sockets in server side ^^

Anyway, I wanted to do my API like this : The user uses these two ws objects (QWsServer, QWsSocket) as he uses these two tcp objects (QTcpServer, QTcpSocket).
The user don't use tcp objects: The TCP instructions are done in background (hidden).

As I said at begining, I have some problems :
- Inherit the QWsServer from QTcpServer : I can't because the newConnection signal is already used in background by the QTcpServer. And I can't offer this signal to the QWsServer users by inheritance.
- Inherit the QWsSocket from QTcpSocket : I can't for the same reason with the readyRead signal.

So I have not used inheritance, but aggregation (I don't know if thats the right word in English). So the QWsServer has a ptr to a QTcpServer as member. And the QWsSocket has a QTcpSocket ptr as member too.
QWsServer inherits from QObject and QWsSocket inherits from QAbstratSocket.
And finaly, for now I created my own signals and fonctions to read data in a QWsSocket, but I wanted to use the QAbstractSocket features to the fullest, and so reimplementing the common read functions and signals.

I hope what I said is clear !

[wysota]

As I said at begining, I have some problems :
- Inherit the QWsServer from QTcpServer : I can't because the newConnection signal is already used in background by the QTcpServer. And I can't offer this signal to the QWsServer users by inheritance.
- Inherit the QWsSocket from QTcpSocket : I can't for the same reason with the readyRead signal.

So I have not used inheritance, but aggregation (I don't know if thats the right word in English). So the QWsServer has a ptr to a QTcpServer as member. And the QWsSocket has a QTcpSocket ptr as member too.
QWsServer inherits from QObject and QWsSocket inherits from QAbstratSocket.
And finaly, for now I created my own signals and fonctions to read data in a QWsSocket, but I wanted to use the QAbstractSocket features to the fullest, and so reimplementing the common read functions and signals.

That's all ok however in my opinion there is just no point in deriving your QWsSocket from QAbstractSocket. QIODevice would do just fine and you could reimplement only those methods you really needed (unfortunately not only the two you mentioned in the beginning).

And here is why I think the way I think... Unless you want to implement the client side as well, your QWsSocket won't sanely implement the following methods from QAbstractSocket:

void	connectToHost ( const QString & hostName, quint16 port, OpenMode openMode = ReadWrite )
void	connectToHost ( const QHostAddress & address, quint16 port, OpenMode openMode = ReadWrite )
QNetworkProxy	proxy () const
void	setProxy ( const QNetworkProxy & networkProxy )
bool	waitForConnected ( int msecs = 30000 )
bool	waitForDisconnected ( int msecs = 30000 )

void	connectToHost ( const QString & hostName, quint16 port, OpenMode openMode = ReadWrite )
void	connectToHost ( const QHostAddress & address, quint16 port, OpenMode openMode = ReadWrite )
QNetworkProxy	proxy () const
void	setProxy ( const QNetworkProxy & networkProxy )
bool	waitForConnected ( int msecs = 30000 )
bool	waitForDisconnected ( int msecs = 30000 )

To copy to clipboard, switch view to plain text mode 

So what is the point in having those methods around?

Correct me if I'm wrong, but what your users need is:

read()
write()
close()
bytesAvailable()
signal readyRead()
signal bytesWritten()
QHostAddress peerAddress()
quint16 peerPort()

read()
write()
close()
bytesAvailable()
signal readyRead()
signal bytesWritten()
QHostAddress peerAddress()
quint16 peerPort()

To copy to clipboard, switch view to plain text mode 

Apart from the last two methods the rest is defined in QIODevice. So you are leaving 6 dangling methods just for the benefit of having other two methods that are pretty simple to implement.

A potential advantage of subclassing QAbstractSocket would be if someone wanted to replace one socket implementation for another. Unfortunately protocols differ so much that in 99% of the cases it is just not possible (and where it is possible, you can just use QIODevice API). So what is the point of deriving from QAbstractSocket?

[erqsor]

I developped an application with my API (So that's a project that uses my websocket server API). And I found usefull to allow server to permit connexion by using one QTcpServer and one QWsServer (So users can connect by two ways : a Qt application that uses TCP protocol, and a web application that uses websocket protocol). And next I stored the socket with a total abstraction of the protocol in a member of my client class (QAbstractSocket * socket) that can stores a QTcpSocket or a QWsSocket (never both). And I want to use the write and read methods with a total abstraction of the type of the socket to simplify and make more powerful the use of the API.
I want users uses the QWsSocket like a QTcpSocket, so I redirect the peerAddress, peerPort, state, socketType functions to the QAbstractSocket (and I need too the connected, disconnected, stateChanged signals). So if my QWsSocket doesn't inherit from QAbstractSocket, I can't use this network part of QIODevice I need.

[wysota]

The abstraction of the protocol probably could have as well used QIODevice instead of QAbstractSocket (since protocols care only about data exchange and not who they are talking to and how). The signal connected() is useless on the server side since when the server accepts a connection, the socket is already in the connected state. Disconnected() is pretty much useless for the server as well and even if not, adding that signal is not a problem. The only two meaningful SocketState values for the server side of the connection are Connected and Closing since the server is accepting the connection so it doesn't do host lookup, it doesn't connect and it's not listening (since it is an incoming connection). However, I'm not going to convince you anymore, it is your project and I just hope people using your code think exactly the same way you do, otherwise you'll get a lot of stupid "why doesn't it work" questions from them.

By the way, there is this thing called QLocalSocket. It's derived from QIODevice and not QAbstractSocket. To be honest I wondered many times why QUdpSocket inherits QAbstractSocket since it's neither connecting nor streaming. I wouldn't be suprised if in Qt5 QAbstractSocket simply vanished into thin air.

[erqsor]

Yes, you said interesting things, but I really want to construct my API like the QTcpSocket and QUdpSocket (and other sockets), That's why I did it like this. I don't want to limit my users. For a personnal project, I will do like you say, but here I think I can't do that :/

Anyway, I did some functions that works:

qint64 QWsSocket::bytesAvailable() const
{
	return buffer.size();
}
 
qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize=buffer.size();
	int i=0;
	while( i<maxSize && i<dataSize )
	{
		data[i] = buffer.dequeue().toAscii();
		i++;
	}
	return i;
}
 
qint64 QWsSocket::writeData( const char * data, qint64 maxSize )
{
	return tcpSocket->write(data, maxSize);
}
 
 
QByteArray QWsSocket::readAll()
{
	qint64 sz = bytesAvailable();
	char * data = new char[sz];
	sz = readData(data, sz);
	return QByteArray(data, sz);
}
 
bool QWsSocket::getChar(char * c)
{
	*c = 0;
	if ( bytesAvailable() == 0 )
		return false;
 
	readData(c, 1);
	return true;
}
 
qint64 QWsSocket::read(char * data, qint64 maxSize)
{
	return readData(data, maxSize);
}

qint64 QWsSocket::bytesAvailable() const
{
	return buffer.size();
}

qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize=buffer.size();
	int i=0;
	while( i<maxSize && i<dataSize )
	{
		data[i] = buffer.dequeue().toAscii();
		i++;
	}
	return i;
}

qint64 QWsSocket::writeData( const char * data, qint64 maxSize )
{
	return tcpSocket->write(data, maxSize);
}


QByteArray QWsSocket::readAll()
{
	qint64 sz = bytesAvailable();
	char * data = new char[sz];
	sz = readData(data, sz);
	return QByteArray(data, sz);
}

bool QWsSocket::getChar(char * c)
{
	*c = 0;
	if ( bytesAvailable() == 0 )
		return false;
	
	readData(c, 1);
	return true;
}

qint64 QWsSocket::read(char * data, qint64 maxSize)
{
	return readData(data, maxSize);
}

To copy to clipboard, switch view to plain text mode 

[wysota]

but I really want to construct my API like the QTcpSocket and QUdpSocket (and other sockets)

What other sockets?

I don't want to limit my users

That's what you're currently doing.

qint64 QWsSocket::bytesAvailable() const
{
	return buffer.size();
}

qint64 QWsSocket::bytesAvailable() const
{
	return buffer.size();
}

To copy to clipboard, switch view to plain text mode 

That's incorrect. See docs for QIODevice::bytesAvailable().

QByteArray QWsSocket::readAll()
{
	qint64 sz = bytesAvailable();
	char * data = new char[sz];
	sz = readData(data, sz);
	return QByteArray(data, sz);
}

QByteArray QWsSocket::readAll()
{
	qint64 sz = bytesAvailable();
	char * data = new char[sz];
	sz = readData(data, sz);
	return QByteArray(data, sz);
}

To copy to clipboard, switch view to plain text mode 

That's incorrect. readAll() is not virtual.

qint64 QWsSocket::read(char * data, qint64 maxSize)
{
	return readData(data, maxSize);
}

qint64 QWsSocket::read(char * data, qint64 maxSize)
{
	return readData(data, maxSize);
}

To copy to clipboard, switch view to plain text mode 

Incorrect, read() is not virtual.

[erqsor]

I don't want to limit my users

That's what you're currently doing.

I'm doing like Qt.

That's incorrect. readAll() is not virtual.

Incorrect, read() is not virtual.

So what can I do ? QIODevice is useless if I cant use this functions...

[wysota]

I'm doing like Qt.

What "Qt does" is not always correct. And you're not "doing like Qt". Qt tends to inherit QIODevice rather than its subclasses.

So what can I do ? QIODevice is useless if I cant use this functions...

Those methods are already implemented, you don't implement them yourself. Implement readData(), writeData(), bytesAvailable(), waitForBytesWritten(), waitForReadyRead(), possibly canReadLine() and readLineData(). QAbstractSocket uses an internal "socket engine" which does most of the work. If you want to implement your own subclass of QAbstractSocket class, you have to shadow all that because you don't have a working socket engine for your protocol underneath since your QWsSocket is not a real native socket. So you have to rewrite pretty much everything QAbstractSocket does (apart from close(), isSequential() and atEnd()).

The more I look at the source code of QAbstractSocket the more I think this class was simply not meant to be subclassed outside Qt tree (not in this particular case but rather not meant to be subclassed at all). It has hardcoded support for UDP, TCP and SSL and doesn't allow hooking into the code with other implementations. QTcpSocket and QUdpSocket are simply stubs over QAbstractSocket which implements everything from both protocols itself (or actually delegates everything to a subclass of QAbstractSocketEngine such as QNativeSocketEngine).

[erqsor]

OK, I deleted these functions:

qint64 read( char * data, qint64 maxSize );
QByteArray readAll();
bool getChar( char * c );

qint64 read( char * data, qint64 maxSize );
QByteArray readAll();
bool getChar( char * c );

To copy to clipboard, switch view to plain text mode 

But that does not work anymore.
The implementation of the others functions, can you take a look ? (only for the read part)

bool QWsSocket::atEnd() const
{
	if ( buffer.size() )
		return false;
	return true;
}
bool QWsSocket::canReadLine() const
{
	return buffer.contains('\n') || QIODevice::canReadLine();
}
bool QWsSocket::isSequential() const
{
	return true;
}
qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	while( i<maxSize && i<dataSize )
	{
		data[i] = buffer.dequeue().toAscii();
		i++;
	}
	return i;
}
qint64 QWsSocket::readLineData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	bool endLineReached = false;
	while( i<maxSize && i<dataSize && !endLineReached )
	{
		char c = buffer.dequeue().toAscii();
		if ( c == '\n' )
			endLineReached = true;
		data[i] = c;
		i++;
	}
	return i;
}

bool QWsSocket::atEnd() const
{
	if ( buffer.size() )
		return false;
	return true;
}
bool QWsSocket::canReadLine() const
{
	return buffer.contains('\n') || QIODevice::canReadLine();
}
bool QWsSocket::isSequential() const
{
	return true;
}
qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	while( i<maxSize && i<dataSize )
	{
		data[i] = buffer.dequeue().toAscii();
		i++;
	}
	return i;
}
qint64 QWsSocket::readLineData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	bool endLineReached = false;
	while( i<maxSize && i<dataSize && !endLineReached )
	{
		char c = buffer.dequeue().toAscii();
		if ( c == '\n' )
			endLineReached = true;
		data[i] = c;
		i++;
	}
	return i;
}

To copy to clipboard, switch view to plain text mode 

---

Added after 9 minutes:

And I must precise that I tried to read data from the QIODevice with readAll and read(bytesAvailable) functions

[wysota]

qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	while( i<maxSize && i<dataSize )
	{
		data[i] = buffer.dequeue().toAscii();
		i++;
	}
	return i;
}

qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	while( i<maxSize && i<dataSize )
	{
		data[i] = buffer.dequeue().toAscii();
		i++;
	}
	return i;
}

To copy to clipboard, switch view to plain text mode 

This is suboptimal (aka slow). Try this:

qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size(); // assuming buffer is QByteArray
	int i = qMin(maxSize, dataSize);
        if(i==0) return 0;
        memcpy(data, buffer.data(), i);
        buffer.remove(0, i);
        return i;
}

qint64 QWsSocket::readData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size(); // assuming buffer is QByteArray
	int i = qMin(maxSize, dataSize);
        if(i==0) return 0;
        memcpy(data, buffer.data(), i);
        buffer.remove(0, i);
        return i;
}

To copy to clipboard, switch view to plain text mode 

qint64 QWsSocket::readLineData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	bool endLineReached = false;
	while( i<maxSize && i<dataSize && !endLineReached )
	{
		char c = buffer.dequeue().toAscii();
		if ( c == '\n' )
			endLineReached = true;
		data[i] = c;
		i++;
	}
	return i;
}

qint64 QWsSocket::readLineData( char * data, qint64 maxSize )
{
	int dataSize = buffer.size();
	int i = 0;
	bool endLineReached = false;
	while( i<maxSize && i<dataSize && !endLineReached )
	{
		char c = buffer.dequeue().toAscii();
		if ( c == '\n' )
			endLineReached = true;
		data[i] = c;
		i++;
	}
	return i;
}

To copy to clipboard, switch view to plain text mode 

Suboptimal. First search for the newline and then read everything in one go.

This all doesn't change the fact that you have to override each and every method implemented by QAbstractSocket because implementations from that class are incompatible with yours.

[erqsor]

Thanks for the optimization.

This all doesn't change the fact that you have to override each and every method implemented by QAbstractSocket because implementations from that class are incompatible with yours.

What do you mean ?

OK, the read and readAll functions I removed they were not virtual, but what should I do instead ? I've already re-implemented all the read functions I can :/

[wysota]

What do you mean ?

See post #29.

OK, the read and readAll functions I removed they were not virtual, but what should I do instead ? I've already re-implemented all the read functions I can :/

Then don't limit yourself to read functions, reimplement the rest as well (you need to shadow all implementation done by QAbstractSocket, including those methods which are introduced by that class. Or... derive from QIODevice instead of QAbstractSocket

[erqsor]

Then don't limit yourself to read functions, reimplement the rest as well (you need to shadow all implementation done by QAbstractSocket, including those methods which are introduced by that class. Or... derive from QIODevice instead of QAbstractSocket

But you will cry if I re-implement functions that are not virtual (in QIODevice or QAbstractSocket).

[wysota]

I never said you were to reimplement any non-virtual methods.

[erqsor]

OK, I had not time to work on it for 1 week. So I will see that when I can.

Anyway, I have one change in my project, somebody asked me to enable the socket client feature. So I think I will enable it soon.

I wanted to speak about another problem. The socketDescriptor functions allow tcp server users to use the socket in another threads. My architecture don't allow this (because my QWsSocket are not real sockets, so I can't give a socketDescriptor of the WsSocket, I can't just give the socketDescriptor of the TcpSocket, but that's not a good idea (I think)). So I need to find a solution. If you have any Idea, that could be cool

[wysota]

Your socket still has a real socket descriptor, it is the TCP socket you are using. So all you need to do is to make sure that moving your socket to another thread also moves the TCP socket with it (for example by making the TCP socket a child of the websocket). So advise your users to use QObject::moveToThread() rather than manipulating the socket descriptor.

[erqsor]

OK, that's a good idea. Anyway I think I can't do that by another way
Thanks

[wysota]

You don't need to do anything at all, to be honest. The socket will do perfectly fine in the same thread along all the other sockets.

[erqsor]

OK, In my actual code, I declare the QWsSocket as the parent of the QTcpSocket, So I think I have nothing to do more. I will test that.