CODEWITHSUNDEEP
cpointersstructglobal-variablesassign
Angga
Angga

Reputation: 115

Change global variable value using pointer

I have problem with my code. my global variable not changed. I have assigned its address to a pointer. This is my struct initialization:

    struct PortData {
        int port;
        int sent;
        int received;
        int total;
        struct PortData *Next;
    };

    struct IPData {
        time_t timestamp;
        uint32_t ip;
        struct PortData Record;
    };

this is my function which returned the address:

    inline struct IPData *FindIp(uint32_t ipaddr) {
    unsigned int counter;

    for (counter = 0; counter < IpCount; counter++)
        if (IpTable[counter].ip == ipaddr)
            return (&IpTable[counter]);

    if (IpCount >= IP_NUM) {
        syslog(LOG_ERR, "IP_NUM is too low, dropping ip....");
        return (NULL);
    }

    memset(&IpTable[IpCount], 0, sizeof (struct IPData));
    IpTable[IpCount].ip = ipaddr;
    return (&IpTable[IpCount++]); 
    }

and here where the pointer assigned to the address of IpTable:

    struct IPData *ptrIPData;
    for (Count = 0; Count < SubnetCount; Count++) {
        if (SubnetTable[Count].ip == (iph->saddr & SubnetTable[Count].mask)) {
            ptrIPData = FindIp(iph->saddr);
            Credit(&(ptrIPData->Record), iph, tcph, srcip);          
        }    
        if (SubnetTable[Count].ip == (iph->daddr & SubnetTable[Count].mask)) {
            ptrIPData = FindIp(iph->daddr);
            Credit(&(ptrIPData->Record), iph, tcph, dstip);
        }

    }

This is my Credit() function:

inline void Credit(struct PortData *pordt, struct iphdr *iph, struct tcphdr *tcph, struct in_addr sipaddr) {

    unsigned int sport, dport;

    memset(&source, 0, sizeof (source));
    source.sin_addr.s_addr = iph->saddr; //init source ip
    sport = ntohs(tcph->source);

    memset(&dest, 0, sizeof (dest));
    dest.sin_addr.s_addr = iph->daddr; //init dest ip
    dport = ntohs(tcph->dest);

    packet_size = ntohs(iph->tot_len);
    if (iph->protocol == 6) //6 is protocol TCP
    {
        prev = portdt;
        int sameport = 0;
        while (prev != NULL) {
            if (dport == prev->port || sport == prev->port) {
                if (dport == prev->port) {
                    prev->sent += packet_size;
                }
                if (sport == prev->port) {
                    prev->received += packet_size;
                }
                sameport = 1;
                break;
            }            
        }

        if (sameport == 0) {
            printf("create new node\n");
            newnode = (struct PortData*) malloc(sizeof (struct PortData));
            newnode->received = 0;
            newnode->sent = 0;
            if (sipaddr.s_addr == source.sin_addr.s_addr) {
                if (tcph->syn == 1 || tcph->ack == 1) {
                    newnode->port = dport;
                    newnode->sent = packet_size;
                }
            }
            if (sipaddr.s_addr == dest.sin_addr.s_addr) {
                if (tcph->syn == 1 && tcph->ack == 1) {
                    newnode->port = sport;
                    newnode->received = packet_size;
                }
            }
            newnode->Next = portdt;
            portdt = newnode;
        }//==end-sameport==
    }//iph->protocol//

    prev = portdt;
    while (prev != NULL) {
        fprintf(logfile, "ip = %s port=%d sent=%d bytes received=%d bytes\n", inet_ntoa(sipaddr), prev->port, prev->sent, prev->received);
        prev = prev->Next;
    }
}

I assumed after executed the Credit() function, value of Iptable[Counter].Record must changed because ptrIPData pointed to it address. but why it does not?

Upvotes: 1

Views: 1681

Answers (2)

Niyoko
Niyoko

Reputation: 7672

Instead of portdt = newnode;, you should replace line with:

portdt->port = newnode->port;
portdt->sent = newnode->sent;
portdt->received = newnode->received;
portdt->total = newnode->total;
portdt->Next = newnode->Next;

Like @David says, portdt = newnode; is only change local variable only, not change something that portdt pointed.

Upvotes: 0

David Heffernan
David Heffernan

Reputation: 612993

You are trying to insert a new item into the linked list Record. You attempt to do this by writing:

newnode = (struct PortData*) malloc(sizeof (struct PortData));
// initialise newnode
newnode->Next = portdt;
portdt = newnode;

The problem is that portdt is local variable that contains the address of the Record member of an IPData struct. So assigning to the local variable portdt achieves nothing that is visible outside the function.

You'll need to make a couple of changes. First of all you need to change struct PortData Record to struct PortData *Record. This is the head pointer of your list, and it must be declared as a pointer to node and not as you have done as a node value.

Then you still pass &Record to Credit, but now portdt is of type struct PortData **Record. So the node insertion assignment becomes

newnode = malloc(sizeof (struct PortData));
// initialise newnode
newnode->Next = *portdt;
*portdt = newnode;

There are other problems with your code. I'm not going to attempt to list them all, but here's what I have seen:

  • Don't cast the return value of malloc.
  • You ought to check the return value of malloc for errors.
  • Declare prev to be a local variable rather than a global as it currently appears to be.

Upvotes: 2

Related Questions