Change global variable value using pointer

Question

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?

A few off topic comments: don't cast the pointer returned by the malloc. You can avoid to write if (tcph->syn == 1 && tcph->ack == 1) twice by moving it. Be consistent in your variable naming: CamelCase or lower? — Maxime Chéramy
– Maxime Chéramy, Commented Jan 9, 2014 at 8:44
while (prev != NULL) can become infinite loop if prev is not NULL before entering loop, because prev is never assigned inside loop. — Niyoko
– Niyoko, Commented Jan 9, 2014 at 10:08
my Credit() function works properly. The number of sent and received has increased by the packet_size when this function executed. okay, I will assigned the prev into NULL. — Angga
– Angga, Commented Jan 9, 2014 at 10:10
Oh sorry, you seems try to insert new value in the beginning of link list, in that case, try @David's answer. — Niyoko
– Niyoko, Commented Jan 9, 2014 at 10:23

David Heffernan · Accepted Answer · 2014-01-09 10:09:45Z

2

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.

answered Jan 9, 2014 at 10:09

David Heffernan

616k46 gold badges1.1k silver badges1.5k bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Angga Over a year ago

thank you for the explanation. then am I need to declare **prev instead of *prev? I need to use prev for searching node inside portdt. if I declared **prev, I don't know the next syntax for the while(prev != NULL), accessing dport = prev->port etc. I'm new in using pointer to pointer like this.

Niyoko · Accepted Answer · 2014-01-09 10:20:05Z

0

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.

answered Jan 9, 2014 at 10:20

Niyoko

7,7124 gold badges35 silver badges60 bronze badges

Collectives™ on Stack Overflow

Change global variable value using pointer

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related