[En-Nut-Discussion] Potential BugFix: tcpsock.c, incorrect pointer management with NutTcpDestroySocket

[Brett Abbott]

Hi

Whilst tracking down a reliability problem, I came across what looks 
like a problem with NutTcpDestroySocket.  It doesnt appear to have been 
managing the linked list correctly.

Could I please ask those more familiar with the OS than I to review the 
original code and my proposed change to see if it is indeed a problem 
and that my proposed fix looks solid.  Of minor note, I have also 
changed the volatile pointer spp back to non volatile as it doesnt 
appear to be needed and may be a hangover from event code where this 
code may have been sourced - again please advise if this is correct.

tcpsock.c appears to have issue in current code levels as well as older 
versions (3.9.5)

Code snippet below.

Many Thanks in advance
Brett

void NutTcpDestroySocket(TCPSOCKET * sock)
{

// Bug Fix Brett Abbott 29Sep2006
//--- Old Code:
//
//    TCPSOCKET *sp;
//    TCPSOCKET *volatile *spp;
//   
//    //@@@printf ("[%04X] Calling destroy.\n", (u_short) sock);
//
//    /*
//     * Remove socket from the list.
//     */
//
//    sp = tcpSocketList;
//    spp = &tcpSocketList;
//    while (sp) {
//        if (sp == sock) {
//            *spp = sp->so_next;
//            break;
//        }
//        spp = &sp->so_next;
//        sp = sp->so_next;
//    }
//
//--- New Code:

    TCPSOCKET *sp;
    TCPSOCKET *spp; // no need to be volatile

    //@@@printf ("[%04X] Calling destroy.\n", (u_short) sock);
    printf ("[%04X] Calling destroy.\n", (u_short) sock);

    /*
     * Remove socket from the list.
     */

    sp = tcpSocketList;
    if(sp==sock) { // first record, special case, only move pointer to 
list rather than altering records

        tcpSocketList=sp->so_next;
   
    } else {

    if (sp) { // Cant assume that there is anything in the list at all, 
avoid linking to null pointer

            spp = tcpSocketList; // Set previous record to the top of 
the list

            sp = sp->so_next; // Set selection pointer to the 2nd in the 
list, if it exists.
            while (sp) {

                if (sp == sock) {

                    spp->so_next = sp->so_next;
                    break;
                }

                spp = sp;
                sp = sp->so_next;
            }
        }
    }   


//--End of Change 29Sep2006

    /*
     * Free all memory occupied by the socket.
     */
    if (sp) {

-- 
-----------------------------------------------------------------
Brett Abbott, Managing Director, Digital Telemetry Limited
Email: Brett.Abbott at digital-telemetry.com
PO Box 24 036 Manners Street, Wellington, New Zealand
Phone +64 (4) 5666-860  Mobile +64 (21) 656-144
------------------- Commercial in confidence --------------------