Calculate IP Address

I created this based on this website which calculates IP address information such as hosts, netmasks etc.

I'm quite new to C and I'm still grasping the idea of the whole array and pointers and bit manipulations. Code works the way I want it to work but I'd like to improve this. I would like to know how I did as well as what are some things I can look out for when programming in C.

```
#include
#include
#include
#include
#include

#define B1 0
#define B2 1
#define B3 2
#define B4 3
#define CIDR 4

void ConvertIpToInt(char address[], int address_block[]);

int main(int argc, char** argv) {

int address_block[4], netmask_block[4];

char address[] = "128.42.0.0/25";

ConvertIpToInt(address, address_block);

char literal_netmask_bin[32];
int bit_counter, net_bit_counter = 0;
while (bit_counter != 33) {
while (net_bit_counter != address_block[CIDR]) {
literal_netmask_bin[bit_counter] = '1';
net_bit_counter += 1;
bit_counter += 1;
}
literal_netmask_bin[bit_counter] = '0';
bit_counter += 1;
}

int i, block_counter, sqN_value, sqN = 0;
bit_counter = 0;
for (i = 0; i [GENERAL]\n\n");
printf("Address:---------------------> %d.%d.%d.%d\n", address_block[B1], address_block[B2], address_block[B3], address_block[B4]);
printf("Network Mask:----------------> %d.%d.%d.%d => %d\n", netmask_block[B1], netmask_block[B2], netmask_block[B3], netmask_block[B4], address_block[CIDR]);
printf("Wildcard Mask:---------------> %d.%d.%d.%d\n", wildcard_netmask_block[B1], wildcard_netmask_block[B2], wildcard_netmask_block[B3], wildcard_netmask_block[B4]);
printf("Network Address:-------------> %d.%d.%d.%d\n", address_block[B1] & netmask_block[B1], address_block[B2] & netmask_block[B2], address_block[B3] & netmask_block[B3], address_block[B4] & netmask_block[B4]);
printf("Broadcast Address:-----------> %d.%d.%d.%d\n", wildcard_netmask_block[B1] | address_block[B1],

Initialize before using

int bit_counter, net_bit_counter = 0;

There are a couple problems. First, if you declare and initialize in the same statement, you should only declare one variable.

int bit_counter = 0;
    int net_bit_counter = 0;

If you have two variables, declare them on two lines. Only declare multiple variables on a single line if you initialize none of them. And some would argue not even then. There's no functional difference, but it is much easier to read with every initialized declaration on a separate line.

Second, you don't initialize bit_counter. C does not automatically initialize variables to default values, so this can cause bizarre behavior. When I first tried to run this code, it gave me a runtime error until I initialized all the variables.

Know your bounds

while (bit_counter != 33) {

This is a bit fragile. You currently allow bit_counter to be incremented multiple times per iteration of this loop. What if when it is 32, you increment to 34 without hitting this check? The check will keep succeeding until the the program crashes (possibly due to trying to write outside the array).

while (bit_counter <= 32) {

This is much more robust. It no longer has to hit the mark exactly to stop. Now it will stop for any value greater than 32, not just 33.

Another benefit, for readability, is that it now says 32, which is the critical number here. Previously it said 33, which is one more than the last number.

But this is actually a bug. It should be

while (bit_counter < 32) {

because literal_bit_mask is only 32 long. The first element is 0. The last element is 31. Now it won't write past the end of the array.

C has an increment by one operator

net_bit_counter += 1;
            bit_counter += 1;

You can write this more idiomatically as

++net_bit_counter;
            ++bit_counter;

This shouldn't make much functional difference (perhaps not any on many platforms), but most C programmers would find this more recognizable.

I used the prefix form because some C compilers may not be smart enough to avoid the copy step of the postfix form. I would expect most compilers to optimize that out, but we don't need to rely on that. It's unlikely to make a noticeable difference here.

++bit_counter increments bit_counter and returns that value. bit_counter++ returns the value it had before incrementing. If you don't use the value (as you don't here), there is no real functional difference. It's possible that the compiler may return different machine instructions. In particular, it might copy the value to another register and then increment. That's unnecessary if the

The following are functionally equivalent (leave the variables with the same values at the end):

int var = bit_counter++;

and

int var = bit_counter;
bit_counter++;

and

int var = bit_counter;
++bit_counter;

The following are different from the previous three, but are functionally equivalent to each other:

bit_counter++;
int var = bit_counter;

and

++bit_counter;
int var = bit_counter;

and

int var = ++bit_counter;

If bit_counter starts as 0 and is incremented to 1, then the first three will set var to 0. The last three will set var to 1.

Many consider it easier to read if the variables are changed on separate lines. This makes it obvious that the variables are changing. Doing two things on the same line is less obvious, because people may see one and miss the other.

StackExchange Code Review Q#141674, answer score: 3