Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

can't use QVector::push_back on QUdpSocket



  • Hi all -

    I'm trying to build a list of sockets that my app is using. Can someone please remind me why this doesn't work:

    QVector<QUdpSocket> myvector;
    QUdpSocket mysock;
    myvector.push_back(mysock);
    

    I get this error:

    C:\qt5\qtbase\src\corelib\tools\qvector.h:792: error: 'QUdpSocket::QUdpSocket(const QUdpSocket&)' is private within this context
             T copy(t);
               ^~~~
    

    Thanks...



  • @mzimmers QObjects are not copyable(see https://doc.qt.io/qt-5/qobject.html#no-copy-constructor-or-assignment-operator), and QUdpSocket is a QObject. The solution is to use a pointer:

    QVector<QUdpSocket*> myvector;
    QUdpSocket *mysock = new QUdpSocket;
    myvector.push_back(mysock);


  • @eyllanesc oh, geez, I keep forgetting that...thanks.

    Now, the QVector is actually a member variable; I need to use its contents outside of the routine this code is in. When the routine is called, it needs to clear the vector first. Do I understand that I need to call clear() and then squeeze() to avoid memory leaks? I'm using 5.14.2.

    Thanks again.



  • @mzimmers
    Before calling clear(), you should assure that the pointers have been freed.
    If not, using qDeleteAll() before clear().



  • @Bonnie ah yes, that's precisely what I need. Thanks, Bonnie.



  • OK, I thought I had a handle on this, but the compiler is telling me differently.

    Background: my app needs to maintain a list of "usable" network interfaces (definition of usable not important here). My thought was to create a struct:

    struct SockInfo
    {
        QNetworkInterface nic;
        QNetworkAddressEntry addr;
        QUdpSocket sockMulticast;
        QUdpSocket sockBroadcast;
    };
    

    and have a QList of these as a member variable. But...when I try to assign a value to an element, I get a compiler error (for the reasons given in prior posts).

    By converting the socket members to pointers, I can do this (this code is executed after the list is created):

    for (it = m_socks.begin(); it != m_socks.end(); ++it)
    {
    	pSockMulticast = new QUdpSocket;
    	SockInfo *p;
    	p = it;
    	p->sockMulticast = pSockMulticast;
    

    But this doesn't seem right -- I don't want a new socket; I want to alter the value of an entry in my list.

    So...I guess I don't really understand this after all. Any explanations, or ideas on doing this better, are most welcome.

    Thanks...


  • Lifetime Qt Champion

    Hi,

    In what way do you want to alter them ?



  • @SGaist one example is that the nic and addr will be set at app startup, but the sockets won't be filled in until later. Plus, if I need to add a connected socket to the struct, and that sock gets disconnected, I need to alter the stored data to reflect that.



  • @mzimmers Use

    struct SockInfo
    {
        QNetworkInterface nic;
        QNetworkAddressEntry addr;
        QUdpSocket *sockMulticast;
        QUdpSocket *sockBroadcast;
    };
    

    On the other hand I recommend you review your basic concepts of C++



  • @eyllanesc said in can't use QVector::push_back on QUdpSocket:

    On the other hand I recommend you review your basic concepts of C++

    Can you elaborate?



  • @mzimmers For example when using the copy constructor, clearly when doing your_struct.sockMulticast = foo you are trying to use the copy constructor since you are trying to copy the foo object to the sockMulticast attribute of the structure. And as I already pointed out the QObjects are not copyable, the copyable ones are the pointers. Or what is the difference of T with T* and understand that it is a pointer, and that you cannot assign a T* to T, nor T to T*.



  • OK, let's do this one step at a time. This is partial code from my header file:

    struct SockInfo
    {
        QNetworkInterface nic;
        QNetworkAddressEntry addr;
        QUdpSocket *sockMulticast;
        QUdpSocket *sockBroadcast;
    };
    class UdpSocket : public QObject
    {
    	QVector<SockInfo> m_socks;
    }
    

    and here's where I create an entry:

    void UdpSocket::getHostAddrs()
    {
            SockInfo sockInfo;
    	sockInfo.nic = *it_qni;
    	sockInfo.addr = addrEntries[i];
    	m_socks.append(sockInfo);
    }
    

    (Obviously some stuff is left out, but I think this demonstrates what I'm attempting.)

    Does this look OK so far?



  • @mzimmers
    Yes :)

    Notice that m_socks.append(sockInfo) copies your local SockInfo sockInfo variable as a new element into m_socks. That's why your QUdpSockets needed to be pointers.

    Though if you'd like us (me) to nitpick (probably not!): reading your code I'd be surprised when I discover a class named UdpSocket holds an array of (pairs of) QUdpSocket, so a whole bunch of QUdpSockets. Maybe at least UdpSockets, or UdpSocketCollection/SockInfoCollection or similar, something to indicate it holds a bunch of them? Perhaps it's just me....



  • @JonB thanks for the confirmation, and I agree entirely about the unfortunate name of this class; that will get remedied.

    In looking over an earlier post of mine, I think I caused some confusion about what I was originally doing. In my struct, I had converted the QUdpSocket elements to pointers; I just didn't post the code. Mea culpa.

    I'm still having some difficulty; unfortunately, my VPN is down, so I can't post code right now. I'll post code when I can, but the essence is, I couldn't make a QVector of SockInfo. I had to make a QVector of Sockinfo *. Apart from seeming like an unnecessary level of indirection, it also made the iterator code uglier than farm subsidies.

    Anyway, like I said, I'll post more code when I can. Thanks...



  • @mzimmers
    My thought would be that the members of SockInfo you show are copyable (pointers to all QObjects now), so I would have expected QVector<SockInfo> m_socks; to be OK.



  • @JonB actually, that's not quite how I did it. I made the QUdpSocket members pointers, but not the other members. Perhaps this was my undoing.



  • @mzimmers Show code when you're ready :)



  • @JonB said in can't use QVector::push_back on QUdpSocket:

    @mzimmers
    My thought would be that the members of SockInfo you show are copyable (pointers to all QObjects now), so I would have expected QVector<SockInfo> m_socks; to be OK.

    You're right -- that is OK; I don't know what my problem was, but it was something else (I need to quit working late at night).

    Here's the code -- preliminary testing indicates it's working, but I invite all critique/comments.

    struct SockInfo
    {
        QNetworkInterface nic;
        QNetworkAddressEntry addr;
        QUdpSocket *sockMulticast;
        QUdpSocket *sockBroadcast;
    };
    class UdpSocket : public QObject
    {
    private:
        QVector<SockInfo> m_socks;
    }
    // creating the list, and populating some of the members.
        QList<QNetworkAddressEntry> addrs; 
        QList<QNetworkAddressEntry>::iterator addr;
    	addrs = nic->addressEntries();
    
    	// for each address entry
    	for (addr = addrs.begin(); addr != addrs.end(); ++addr)
    	{
    		qha = addr->ip();
    		if (qha.protocol() == QAbstractSocket::IPv4Protocol) // currently ignoring IP6 addresses.
    		{
    			sockInfo = new SockInfo;
    			sockInfo->nic = *nic;
    			sockInfo->addr = *addr;
    			m_socks.append(*sockInfo);
    			break;
    		}
    	}
    // updating the list.
        QVector<SockInfo>::iterator sock;
        for (sock = m_socks.begin(); sock != m_socks.end(); ++sock)
        {
            pSockMulticast = new QUdpSocket;
            pSockBroadcast = new QUdpSocket;
            sock->sockMulticast = pSockMulticast;
            sock->sockBroadcast = pSockBroadcast;
    ...
    

    Thanks...



  • @mzimmers
    Briefly:

            sockInfo = new SockInfo;
            m_socks.append(*sockInfo);
    

    Unless the sockinfo variable used here is a member variable of the class and you delete it in class destructor, or maybe you delete it in omitted code, this leaks. The sockinfo = new SockInfo instance is not itself being put in m_socks with m_socks.append(*sockInfo);, that copies it. Why do you new your SockInfos at all? If you are not intending this, you could have had:

    QSockInfo sockinfo;
    m_socks.append(sockInfo);
    

    It's OK to use a local/stack variable which goes out of scope because m_socks.append(sockInfo) is storing a copy of it.

    Separately

            sock->sockMulticast = pSockMulticast;
            sock->sockBroadcast = pSockBroadcast;
    

    If you are updating, what are you doing about the sock->sockMulticast/Broadcast which is currently there, which itself was previously newed here & elsewhere? Maybe that's in your omitted code.



  • I can see that I need to rename several variables...it seems like I actually tried to make this confusing.

    I left out the code that ties all this together. Whenever the main routine is invoked, it first calls this:

    void UdpSocket::cleanup()
    {
        QVector<SockInfo>::iterator it;
    
        for (it = m_socks.begin(); it != m_socks.end(); ++it)
        {
            if (it->sockMulticast->isValid())
            {
                delete it->sockMulticast;
            }
            if (it->sockBroadcast->isValid())
            {
                delete it->sockBroadcast;
            }
        }
        qDeleteAll(m_socks.begin(), m_socks.end());
        m_socks.clear();
    

    ...and then calls the code that creates the list, and then calls the code that creates the QUdpSockets that go into that list. So, I think I'm good on memory leaks (but of course I could be wrong).

    Your point about using a struct instead of a pointer to a struct is well-taken; I've changed that.

    Thanks...

    EDIT: since no one else commented on my implementation, I'm going to assume that it's tolerable. Marking thread as solved. Thanks to all who helped.


Log in to reply