Scanning the subnet by ip range

Question

By applying the get_name and get_mac functions to the specified IP address, it is possible to estimate the network availability of a segment:

#include <Winsock2.h>
#include <iphlpapi.h>
#include <cstdio>
#pragma comment(lib, "iphlpapi.lib")
#pragma comment(lib, "ws2_32.lib")
bool get_name(unsigned char* name, char dest[32])
{ 
 struct in_addr destip;
 struct hostent* info;
 destip.s_addr = inet_addr(dest);
 info = gethostbyaddr((char*)&destip, 4, AF_INET);
 if (info != NULL)
{
 strcpy((char*)name, info->h_name);
}
else
{
return false;
}
return true;
}
bool get_mac(unsigned char* mac, char dest[32])
{
 struct in_addr destip;
 ULONG mac_address[2];
 ULONG mac_address_len = 6;
 destip.s_addr = inet_addr(dest);
 SendARP((IPAddr)destip.S_un.S_addr, 0, mac_address, &mac_address_len);
 if (mac_address_len)
 {
 BYTE* mac_address_buffer = (BYTE*)&mac_address;
 for (int i = 0; i < (int)mac_address_len; i++)
 {
 mac[i] = (char)mac_address_buffer[i];
 }
 }
 else
 {
 return false;
 }
 return true;
 }
 int main()
 {
 char address[][32] = { { "192.168.14.101" },{ "192.168.14.102" }, 
 {"192.168.14.103" },{ "192.168.14.106 "} };
 WSADATA sock;
 if (WSAStartup(MAKEWORD(2, 2), &sock) != 0)
 {
 printf("Failed to initialise winsock. (%d)\n", WSAGetLastError());
 return 1;
 }
 for (int i = 0; i < (int)sizeof(address) / 32; i++)
 {
 unsigned char mac[6] = { '0円' };
 unsigned char name[100] = { '0円' };
 if (get_mac(mac, address[i]))
 {
 printf("%s : %s : %.2X-%.2X-%.2X-%.2X-%.2X-%.2X\n", address[i], 
 (get_name(name, address[i])) ? (char*)name : "-", 
 mac[0],mac[1],mac[2],mac[3], mac[4], mac[5]);
 fflush(stdout);
 }
 }
 printf("\nDone.\n");
 fflush(stdout);
 system("PAUSE");
 return 0;
 }

An attempt to implement a program that scans the subnet and displays IP address, host name, and MAC of machines on the network.

Answer 1

I won't cover any bugs in your implementation, I'll only have a look at your coding style. It looks like you were writing plain c but tagged it as c++. I think I didn't even found a single c++-only feature you made usage of.

Don't use a string as data storage unless it's necessary

And of course, don't use plain c-style strings, because you'll do it wrong.

char address[][32] = { { "192.168.14.101" },{ "192.168.14.102" }, 
 {"192.168.14.103" },{ "192.168.14.106 "} };

You reserve 32 chars for your ip-address storage, but that isn't ever necessary. If you really want to store them as a string, do it as std::string, or if it's a constant string, as std::string_view(C++17). An ip-address has a upper maximum of 4 * 3 signs + 3 dot signs and additionally the zero at the end (that accumulates to a max of 16 signs). So, that means you reserved twice as much memory as you really ever need.

Use better data type

Why don't you simply store the ip-address as an 32bit uint; or as an std::array<std::unit8_t, 4>. You could easily create a typedef for that: using ipv4 = std::array<std::unit8_t, 4> With that typedef you'll be able to use that type everywhere you need the ipv4 address. That makes the code more readable and less error prone.

rethink your return statements

That looks really horrible.

bool get_name(unsigned char* name, char dest[32])
{
 // ...
 if (info != NULL)
 {
 // ...
 }
 else
 {
 return false;
 }
 return true;
}

You could write that in an easier way:

bool get_name(unsigned char* name, char dest[32])
{
 // ...
 if (info != NULL)
 {
 // ...
 return true;
 }
 return false;
}

use std::optional if you expect your function return a invalid return value

In C++17 we got a new helper template class called std::optional. It's very helpfull in returning values with an invalid state. You could adjust your get_name function like this:

std::optional<std::string> get_name(ipv4 ip)
{
 // ...
 if (info != NULL)
 {
 return info.name; // pseudo-code; don't know the real interface of this info object, but hopefully you get what I try to explain
 }
 return std::nullopt;
}

That will make your code more reable and self-descriptive. All of the above is also true for your get_mac function.

c-style cast are bad

You used them alot; don't do that. In c++ there are plenty of cast you should use instead. For example (int)mac_address_len should be written as static_cast<int>(mac_address_len)