Network utility tool

Question

I wrote a little program in C based on some requirements:

• Ping every IP from a file and check the result
• Show, by network adapter, ONLY the IP, subnet, default gateway and DNS
• Check the speed between two pings.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAX_LEN 256
void readAndPing(char* path) {
 /*
 Lee el contenido del fichero indicado por path
 y lanza pings a cada una de las direcciones ip del fichero.
 Printa si el ping ha sido efectivo o no.
 */
 printf("Reading file %s...\n", path);
 FILE* fp = fopen(path, "r");
 if (fp == NULL) {
 printf("File not found!\n");
 }
 else {
 char buffer[MAX_LEN];
 while (fgets(buffer, MAX_LEN, fp)) {
 buffer[strcspn(buffer, "\n")] = 0;
 char comando[MAX_LEN];
 snprintf(comando, sizeof(comando), "ping -n 1 %s > NUL", buffer);
 printf("Executing command %s\n", comando);
 int ping_res = system(comando);
 if (ping_res) printf("Ping failed!\n");
 else printf("Pinged!\n");
 }
 fclose(fp);
 }
}
void showConfigNetAdpt(char* adptName) {
 /*
 Muestra por pantalla la configuración de red del adaptador adptNAme
 creando un archivo netAdpt.txt y mostrando sus resultados.
 */
 char comando[MAX_LEN];
 snprintf(comando, sizeof(comando),
 "netsh interface ip show config %s > netAdpt.txt", adptName);
 printf("Executing command %s\n", comando);
 int res = system(comando);
 if (res) {
 printf("Command failed!\n");
 }
 else {
 //Lectura del fichero creado
 FILE* fp = fopen("netAdpt.txt", "r");
 if (fp == NULL) {
 printf("File not found\n");
 }
 else {
 char buffer[MAX_LEN];
 while (fgets(buffer, MAX_LEN, fp)) {
 //Elimina el newline
 buffer[strcspn(buffer, "\n")] = 0;
 char* line;
 //Cosas de no tener utf-8...
 line = strstr(buffer, "Direcci¢n IP:");
 if (line) printf("%s\n", line);
 line = strstr(buffer, "Prefijo de subred:");
 if (line) printf("%s\n", line);
 line = strstr(buffer, "Puerta de enlace predeterminada:");
 if (line) printf("%s\n", line);
 line = strstr(buffer, "Servidores DNS configurados a trav‚s de DHCP:");
 if (line) {
 printf("%s\n", line);
 fgets(buffer, MAX_LEN, fp);
 line = strstr(buffer, "Registrar con el sufijo:");
 while (line == NULL) {
 printf("\t%s\n", buffer);
 fgets(buffer, MAX_LEN, fp);
 line = strstr(buffer, "Registrar con el sufijo:");
 }
 }
 }
 fclose(fp);
 }
 }
}
void printResultsPingMean(char* path, char* ip) {
 FILE* fp = fopen(path, "r");
 printf("Results from %s\n", ip);
 char buffer[MAX_LEN];
 char* line;
 if (fp == NULL) {
 printf("File not foudn\n");
 }
 else {
 while (fgets(buffer, MAX_LEN, fp)) {
 //Elimina el newline
 buffer[strcspn(buffer, "\n")] = 0;
 line = strstr(buffer, "Media ");
 if (line != NULL) {
 printf("\t%s\n", line);
 }
 }
 fclose(fp);
 }
}
void fastestDNS(char* ip1, char* ip2) {
 char comando[MAX_LEN];
 snprintf(comando, sizeof(comando),
 "ping -n 5 %s > pingResDNS1.txt", ip1);
 printf("Executing command %s...\n", comando);
 int e_c1 = system(comando);
 if (e_c1) {
 printf("Error executing command! Is %s a valid IP? !\n", ip1);
 }
 snprintf(comando, sizeof(comando),
 "ping -n 5 %s > pingResDNS2.txt", ip2);
 printf("Executing command %s...\n", comando);
 int e_c2 = system(comando);
 if (e_c2) {
 printf("Error executing command! Is %s a valid IP? !\n", ip2);
 }
 printResultsPingMean("pingResDNS1.txt", ip1);
 printResultsPingMean("pingResDNS2.txt", ip2);
}
void showMenu() {
 printf("-------------------------------------------------------\n");
 printf("Net Utilities\n");
 printf("1: Ping every IP from a file\n");
 printf("2: Check net config by adapter\n");
 printf("3: Compare ping speed of two IP's\n");
 printf("4: Exit\n");
 printf("-------------------------------------------------------\n");
}
int readOption() {
 showMenu();
 int opcion = -1;
 scanf("%d", &opcion);
 while (opcion < 1 || opcion > 4) {
 printf("Invalid option!\n");
 showMenu();
 scanf("%d", &opcion);
 }
 return opcion;
}
int main() {
 int opcion = -1;
 char buffer[MAX_LEN], buffer2[MAX_LEN];
 while (opcion != 4) {
 opcion = readOption();
 printf("Option %d\n", opcion);
 if (opcion == 1) {
 printf("\n-------------------------------------------------------\n");
 printf("Ping every IP from a file\n");
 printf("-------------------------------------------------------\n");
 printf("Filename: ");
 scanf("%s", buffer);
 readAndPing(buffer);
 }
 if (opcion == 2) {
 printf("\n-------------------------------------------------------\n");
 printf("Check net config by adapter\n");
 printf("-------------------------------------------------------\n");
 printf("Adapter name: ");
 scanf("%s", buffer);
 showConfigNetAdpt(buffer);
 }
 if (opcion == 3) {
 printf("\n-------------------------------------------------------\n");
 printf("Compare ping speed of two IP's\n");
 printf("-------------------------------------------------------\n");
 printf("First IP: ");
 scanf("%s", buffer);
 printf("Second IP: ");
 scanf("%s", buffer2);
 fastestDNS(buffer, buffer2);
 }
 printf("\n\n");
 }
}

I'm using windows and my default language is spanish, so that's why the strstr calls are strange... However, I would like some feedback. This is the first time I used windows commands inside C.

Answer 1

Nicely formatted

comando[] deserves a greater size

 char buffer[MAX_LEN];
 ...
 //char comando[MAX_LEN];
 //snprintf(comando, sizeof(comando), "ping -n 1 %s > NUL", buffer);
 #define FMT_PING "ping -n 1 %s > NUL"
 char comando[MAX_LEN + sizeof(FMT_PING)];
 snprintf(comando, sizeof(comando), FMT_PING, buffer);

Line size

256 is not so generous for a command line size. Consider BUFSIZ from <stdio.h> "size of the buffer used by the setbuf function"

// #define MAX_LEN 256
#define MAX_LEN BUFSIZ
// or ....
#define MAX_LEN 4096 // If you want a fixed size

Beware of variant text file line endings

Sometimes a system will read a text file that is foreign to the local OS with "\n" and "\r\n" expected - or visa-versa.

Consider lopping off line ends, be they "\n", "\r\n","\r".

// buffer[strcspn(buffer, "\n")] = 0;
buffer[strcspn(buffer, "\n\r")] = 0;

Check return values

Check return values for errors, especially input functions.

// scanf("%d", &opcion);
if (scanf("%d", &opcion) != 1) {
 TBD_CODE_Handle_Error();
}

scanf("%s", ... is worse than gets()

Use a width limit.
Better yet, use fgets().

Better output

With the below, the end of the command is unclear. Consider sentinels like below. Issue in multiple places throughout code.

// printf("Executing command %s...\n", comando);
printf("Executing command \"%s\"...\n", comando);

Check spelling

//"File not foudn\n"
"File not found\n"

Overall error handling

In various places, code detects an error and must promptly stop, yet main() only returns 0.

Consider returning EXIT_FAILURE from main() in those cases.

Rather then print errors to stdout, consider stderr.

// printf("File not found\n");
fprintf(stderr, "File not found\n");

Language

Code uses hard coded strings like "Direcci¢n IP:", unlikely to work in many locales. Consider internationalization.

Answer 2

Using system and piping the results into a temp file can get you in trouble. What happens if you do not have the correct privileges to write to the current folder? Running more than one instance of your program at the same time?

The correct, but rather long and boring, way to do this on Windows is to use CreateProcess. Here is a basic example of how to use it:

#include <windows.h> 
#include <stdio.h>
#define MAX_CMDLINE 8191 
#define BUFFER_SIZE 4096
BOOL executeAndGetResults(LPCSTR currentDirectory, LPCSTR applicationName, LPCSTR commandLine, LPSTR buffer, DWORD buflen, DWORD* exitCode)
{
 BOOL ok = TRUE;
 HANDLE stdInPipeRead = NULL;
 HANDLE stdInPipeWrite = NULL;
 HANDLE stdOutPipeRead = NULL;
 HANDLE stdOutPipeWrite = NULL;
 // Create two pipes.
 SECURITY_ATTRIBUTES sa = { sizeof(SECURITY_ATTRIBUTES), NULL, TRUE };
 if (!CreatePipe(&stdInPipeRead, &stdInPipeWrite, &sa, 0))
 {
 fprintf(stderr, "Failed to create input pipes\n");
 return FALSE;
 }
 if (!CreatePipe(&stdOutPipeRead, &stdOutPipeWrite, &sa, 0))
 {
 CloseHandle(stdInPipeRead);
 CloseHandle(stdInPipeWrite);
 fprintf(stderr, "Failed to create output pipes\n");
 return FALSE;
 }
 // copy command line to non-constant string
 char commandLineCopy[MAX_CMDLINE];
 strcpy_s(commandLineCopy, MAX_CMDLINE, commandLine);
 // Create the process.
 STARTUPINFO si;
 memset(&si, 0, sizeof(si));
 si.cb = sizeof(STARTUPINFO);
 si.dwFlags = STARTF_USESTDHANDLES;
 si.hStdError = stdOutPipeWrite;
 si.hStdOutput = stdOutPipeWrite;
 si.hStdInput = stdInPipeRead;
 PROCESS_INFORMATION pi;
 memset(&pi, 0, sizeof(pi));
 LPSECURITY_ATTRIBUTES processAttributes = NULL;
 LPSECURITY_ATTRIBUTES threadAttribute = NULL;
 BOOL inheritHandles = TRUE;
 DWORD creationFlags = 0;
 LPVOID environment = NULL;
 if (!CreateProcessA(applicationName,
 commandLineCopy,
 processAttributes,
 threadAttribute,
 inheritHandles,
 creationFlags,
 environment,
 currentDirectory,
 &si,
 &pi))
 {
 CloseHandle(stdInPipeRead);
 CloseHandle(stdInPipeWrite);
 CloseHandle(stdOutPipeRead);
 CloseHandle(stdOutPipeWrite);
 fprintf(stderr, "Failed to create process\n");
 return FALSE;
 }
 // Close pipes we do not need.
 CloseHandle(stdOutPipeWrite);
 CloseHandle(stdInPipeRead);
 // The main loop for reading output from the command
 DWORD read = 0;
 char* ptr = buffer;
 while (ReadFile(stdOutPipeRead, ptr, buflen, &read, NULL))
 {
 ptr += read;
 buflen -= read;
 }
 *ptr = '0円';
 // Clean up and exit.
 CloseHandle(stdOutPipeRead);
 CloseHandle(stdInPipeWrite);
 GetExitCodeProcess(pi.hProcess, exitCode);
 CloseHandle(pi.hProcess);
 CloseHandle(pi.hThread);
 return TRUE;
}
int main()
{
 printf("pinging google...\n");
 char systemPath[_MAX_PATH];
 GetSystemDirectoryA(systemPath, _MAX_PATH);
 char pingPath[_MAX_PATH];
 snprintf(pingPath, _MAX_PATH, "%s\\ping.exe", systemPath);
 
 char buffer[BUFFER_SIZE + 1];
 DWORD retcode = 0;
 if (!executeAndGetResults(NULL, pingPath, "ping.exe www.google.com", buffer, BUFFER_SIZE, &retcode))
 return EXIT_FAILURE;
 printf("retcode: [%d]\n", retcode);
 printf("results: [%s]\n", buffer);
}

• chux: Code uses strcpy_s(), snprintf() to avoid buffer overflow, yet lacks handling cases where the buffer was not long enough. Instead, code goes on as if no problem. Likewise, various functions errors are not check as in GetSystemDirectoryA(). Perhaps simply EXIT_FAILURE on error?