Wi-fi scheduler written in C

Question

I'm new to C and programming in general (some minimal prior exposure to JS and python). This is also my first time working with shell commands, scripts, and cron. This program works on my macOS and Lubuntu running on a VM (so long as you run as root).

Using this program, you can set times throughout the day when your wifi turns off automatically. When your wifi is off, you can still manually turn on wifi for urgent situations for a short time (default is 30 seconds) before the program automatically turns off the wifi again.

The code is separated into 2 C programs and 1 shell script.

checkon.sh (runs in bg and turns off wifi after a short time if it detects it was turned on)

#!/bin/bash

# Check OS
if [[ "$OSTYPE" == "linux-gnu"* ]]; then
        # If wifi on for ubuntu, keyword is UP
        wifion=UP
elif [[ "$OSTYPE" == "darwin"* ]]; then
        # Name of Wi-Fi device is en0
        device=en0
        # If wifi on for osx, keyword is active
        wifion=active
fi

# Check status of wi-fi device (is it active or inactive?)
status() {
        if [[ "$OSTYPE" == "linux-gnu"* ]]; then
                ip l show enp0s3 | awk '/state/{print $9}'
        elif [[ "$OSTYPE" == "darwin"* ]]; then
                /sbin/ifconfig $device | awk '/status:/{print $2}'
        fi
}

# If wifi is on, turn off after X seconds
isactive() {
        sleep 30
        if [[ "$OSTYPE" == "linux-gnu"* ]]; then
                nmcli networking off
        elif [[ "$OSTYPE" == "darwin"* ]]; then
                /usr/sbin/networksetup -setairportpower $device off
        fi
}

# Every 5 seconds, check if wifi is active or inactive
while :; do
        if [[ "$(status)" == "$wifion" ]]
        then isactive
        fi
        sleep 5
done

setcheck.c (cron will launch this program)

#include <stdio.h>  // For strerror()
#include <stdlib.h> // For exit() and system()
#include <string.h> // For strcmp()
#include <unistd.h> // For fork(), getcwd()
#include <errno.h>  // For errno
#include <time.h>   // For time()

// Determine os
#if __APPLE__
    #define PLATFORM_NAME "apple"
#elif __linux__
    #define PLATFORM_NAME "linux"
#endif

#define MAXLINE 1000

// 1) If I don't assign macro to a variable, compiler will warn that code
// strcmp(PLATFORM_NAME, "apple") will result in unreachable code. 
// Need to determine condition at run-time vs compile-time.
// 2) Need `static` to avoid -Wmissing-variable-declaration warning
static char os[MAXLINE];

int startcheck(void);
int endcheck(void);
int pmessages(char *message);
int turnonwifi(void);
int turnoffwifi(void);

// Cron calls main(), telling it to either turn on wifi (normal state)
// or turn off wifi (run script)
int main(int argc, char *argv[]) {
    strcpy(os, PLATFORM_NAME);
        if (argc == 1) {
                printf("Include argument 'wifion' or 'wifioff'\n");
                return 1;
        }

        // Turn off or on the background script
        if (!strcmp(argv[1], "wifion")) {
                pmessages("Turning on wifi");
                turnonwifi();
                // Run program shuts down checkon script
                if (endcheck())
                        return 1;
        } else if (!strcmp(argv[1], "wifioff")) {
                pmessages("Turning off wifi"); 
                turnoffwifi();
                if(startcheck())
                        return 1;
        } else {
                pmessages("Error: Argument must be 'wifion' or 'wifioff': %s\n");
    }
        return 0;
}

// Launch checkon script
int startcheck(void) {
    char cwd[MAXLINE];
    getcwd(cwd, MAXLINE);

    char *checkon_path2 = "/checkon.sh";
    char *checkon_path = strcat(cwd, checkon_path2);
    char *checkon_arg0 = "checkon.sh";

    // Check if checkon.sh already exists, if so, print error and exit
    // Need to use '-f' and include '.sh' in search for osx
    int isrunning = 1;
    if (!strcmp(os, "apple")) {
            isrunning = system("/usr/bin/pgrep -f checkon.sh >>/dev/null 2>>/dev/null");
    } else if (!strcmp(os, "linux")) {
            // NTD: Why does system() return 0 when '-f' is used in
            // ubuntu when process doesn't exist?
            isrunning = system("/usr/bin/pgrep checkon >>/dev/null 2>>/dev/null");
    } else {
            printf("Warning: cannot determine if checkon.sh is running\n");
    }
    if(isrunning == 0) {
            pmessages("Error: checkon.sh already exists");
            exit(1);
    }

    // Fork creates a child process that is identical except it returns
    // 0 for the child and the pid of the child for the parent process
    int pid = fork();
    // We fork so that child runs in background when parent exits
    if (pid == 0) {
            setsid();
            execl(checkon_path, checkon_arg0, (char *) NULL);
        if (errno) {
            printf("errno %d: %s\n", errno, strerror(errno));
            return 1;
        }
    } else {
        exit(0);
    }
    return 0;
}

// Find checkon script and kill it
int endcheck(void) {    
    // pgrep -f finds name of script and kill will end it.
    // Redirect output otherwise if checkon.sh doesn't exist, 
    // command will print error to terminal
    if (!strcmp(os, "apple")) {
            system("/bin/kill $(/usr/bin/pgrep -f checkon.sh) >>/dev/null 2>>/dev/null");
    } else if (!strcmp(os, "linux")) {
            system("/usr/bin/pkill checkon >>/dev/null 2>>/dev/null");
    }
    return 0;
}

// Print messages
int pmessages(char *pmessage) {
    time_t current_time = time(NULL);
    char *c_time_string = ctime(&current_time);

    if (pmessage != NULL) {
            printf("%s: %s", pmessage, c_time_string);
            return 0;
    } else {
            printf("pmessages error: no message: %s\n", c_time_string);
            return 1;
    }
}

// Turn on wifi
int turnonwifi() {
    // Include full path b/c cron sets a different environment 
    // than shell, meaning PATH variable is different
    if (!strcmp(os, "apple")) {
            system("/usr/sbin/networksetup -setairportpower en0 on");
    } else if (!strcmp(os, "linux")) {
            system("nmcli networking on");
    }
    return 0;
}

// Turn off wifi
int turnoffwifi() {
    // Include full path b/c cron sets a different environment 
    // than shell, meaning PATH variable is different
    if (!strcmp(os, "apple")) {
            system("/usr/sbin/networksetup -setairportpower en0 off");
    } else if (!strcmp(os, "linux")) {
            system("nmcli networking off");
    } 
    return 0;
}

swifi.c (user interacts with this program to set cron jobs)

#include <stdio.h>  // For printf()
#include <stdlib.h> // For system()
#include <string.h> // For strcmp()
#include <inttypes.h>   // For strtol()
#include <unistd.h> // For getopt() and getcwd()
#include <errno.h>  // For errno
#include <time.h>   // For time(), ctime(), etc.

// Determine os
#if __APPLE__
    #define PLATFORM_NAME "apple"
#elif __linux__
    #define PLATFORM_NAME "linux"
#endif
#define MAXLINE 1000

struct hourmin {
    int minute;
    int hour;
};

// Need static keyword to avoid -Wmissing-variable-declaration warning
static char cwd[MAXLINE];
static char os[MAXLINE];
static int addflag, rmvflag;
// Default: Be on the whole day
static struct hourmin starttime = { 1, 0 };
static struct hourmin endtime = { 0, 0 };

void getarg(int argc, char **argv);
int addcron(struct hourmin *starttime, struct hourmin *endtime);
int rmvcron(struct hourmin *starttime, struct hourmin *endtime);
int clearcron(void);
int turnoffwifi(void);
int turnonwifi(void);
int gethourmin(char *strtime, struct hourmin *);
int istimebetween(void);

// Takes 3 arguments: 1) add or rmv, 2) start time (-s) and 3) end time (-e)
int main(int argc, char *argv[]) {
    getcwd(cwd, MAXLINE);
    strcpy(os, PLATFORM_NAME);

    if (argc == 1) {
        printf("Error: Provide at least one argument 'add' or 'rmv'\n");
        return 1;
    }

    getarg(argc, argv);

    if (addflag) {
        addcron(&starttime, &endtime);
        if(istimebetween())
            turnoffwifi();
        printf("Wifi set to turn off at %d:%02d\n", starttime.hour, starttime.minute);
        printf("Wifi set to turn on at %d:%02d\n", endtime.hour, endtime.minute);
    } else if (rmvflag) {
        rmvcron(&starttime, &endtime);
        if(istimebetween())
            turnonwifi();
        printf("Timer removed. Start: %d:%02d End: %d:%02d\n", starttime.hour, starttime.minute, endtime.hour, endtime.minute);
    } 

    return 0;
}

// Parse arguments using getopt(). Need a separate function 
// to handle GNU / Linux case and OSX / BSD case.
void getarg(int argc, char **argv) {
    int ch;     // getopt man page uses "ch"
    addflag = rmvflag = 0;

    if (!strcmp(os, "apple")) {
        while (optind < argc) {
            if ((ch = getopt(argc, argv, "s:e:")) != -1) {
                switch (ch) {
                case 's':
                    gethourmin(optarg, &starttime);
                    break;
                case 'e':
                    gethourmin(optarg, &endtime);
                    break;
                default:
                    break;
                }
            } else if (!strcmp(argv[optind], "add")) {
                addflag = 1;
                optind++;
            } else if (!strcmp(argv[optind], "rmv")) {
                rmvflag = 1;
                optind++;
            } else if (!strcmp(argv[optind], "clear")) {
                clearcron();
                optind++;
            } else {
                printf("Error: Unrecognized argument.\n");
                exit(1);
            }
        }
    } else if (!strcmp(os, "linux")) {
        while ((ch = getopt(argc, argv, "s:e:")) != -1) {
            switch (ch) {
            case 's':
                gethourmin(optarg, &starttime);
                break;
            case 'e':
                gethourmin(optarg, &endtime);
                break;
            default:
                break;
            }
        }
        int index;
        for (index = optind; index < argc; index++) {
            if (!strcmp(argv[optind], "add")) {
                addflag = 1;
            } else if (!strcmp(argv[optind], "rmv")) {
                rmvflag = 1;
            } else if (!strcmp(argv[optind], "clear")) {
                clearcron();
            } else {
                printf("Error: Unrecognized argument.\n");
                exit(1);
            }
        }

    }
}

// Adds cronjob to crontab
int addcron(struct hourmin *start_time, struct hourmin *end_time) {
    char cron_wifioff[MAXLINE];
    char cron_wifion[MAXLINE];

    // NTD: Is there a cleaner way of preparing these commands?
    sprintf(cron_wifioff, "(crontab -l 2>>/dev/null; echo '%d %d * * * %s/setcheck.out wifioff >>/dev/null 2>>/dev/null') | sort - | uniq - | crontab -", start_time->minute, start_time->hour, cwd);
    sprintf(cron_wifion, "(crontab -l 2>>/dev/null; echo '%d %d * * * %s/setcheck.out wifion >>/dev/null 2>>/dev/null') | sort - | uniq - | crontab -", end_time->minute, end_time->hour, cwd);

    system(cron_wifioff);
    system(cron_wifion);
    return 0;
}

// Removes cronjobs from crontab
int rmvcron(struct hourmin *start_time, struct hourmin *end_time) {
    // rcron stands for remove cron
    char rcron_wifioff[MAXLINE];
    char rcron_wifion[MAXLINE];

    // NTD: Is there a cleaner way of preparing these commands?
    sprintf(rcron_wifioff, "(crontab -l | sort - | uniq - | grep -v '%d %d \\* \\* \\* %s/setcheck.out wifioff >>/dev/null 2>>/dev/null') | crontab -", start_time->minute, start_time->hour, cwd);
    sprintf(rcron_wifion, "(crontab -l | sort - | uniq - | grep -v '%d %d \\* \\* \\* %s/setcheck.out wifion >>/dev/null 2>>/dev/null') | crontab -", end_time->minute, end_time->hour, cwd);

    system(rcron_wifioff);
    system(rcron_wifion);
    return 0;
}

// Clears crontab and restarts wifi
int clearcron(void) {
    system("crontab -r");
    turnonwifi();
    return 0;
}

int turnoffwifi(void) {
    char wificmd[MAXLINE];
    sprintf(wificmd, "%s/setcheck.out wifioff >>/dev/null 2>>/dev/null", cwd);
    system(wificmd);
    return 0;
}

int turnonwifi(void) {
    char wificmd[MAXLINE];
    sprintf(wificmd, "%s/setcheck.out wifion >>/dev/null 2>>/dev/null", cwd);
    system(wificmd);
    return 0;
}

// Get current time and compare if it is in between
// NTD: Not sure what to do if starttime = endtime. cronjob race condition?
int istimebetween(void) {
    // Get current time into int format (tm struc that contains int members) 
    time_t current_time = time(NULL);
    struct tm *current_tm = localtime(&current_time);

    // Rename for ease
    int chour = current_tm->tm_hour;
    int cmin = current_tm->tm_min;
    int shour = starttime.hour;
    int smin = starttime.minute;
    int ehour = endtime.hour;
    int emin = endtime.minute;

    // If endtime > starttime, check if current time is in between
    // If endtime < starttime, check if current time is NOT in between
    int checkbetween = 0;
    if ((ehour > shour) || (ehour == shour && emin > smin))
        checkbetween = 1;

    // Compare current time to starttime and endtime
    int isbetween = 0;
    switch (checkbetween) {
    case 0:
        // In case 0, endtime is "earlier" than starttime, so 
        // we check endtime < currenttime < starttime. 4 possibilities:
        // 1) endhour < currenthour < starthour
        // 2) endhour = currenthour < starthour; endmin < currentmin
        // 3) endhour < currenthour = starthour; currentmin < startmin
        // 4) endhour = currenthour = starthour; endmin < cmin < startmin
        if ((chour > ehour && chour < shour)
            || (chour == ehour && cmin > emin && chour < shour)
            || (chour == shour && cmin < smin && chour > ehour)
            || (chour == ehour && chour == shour 
                && cmin > emin && cmin < smin))
            isbetween = 1;
        break;
    case 1:
        // In case 1, end time is "later" than starttime, so
        // we check starttime < currenttime < endtime. Inverse of above.
        if ((chour > shour && chour < ehour)
            || (chour == shour && cmin > smin && chour < ehour)
            || (chour == ehour && cmin < emin && chour > shour)
            || (chour == shour && chour == ehour 
                && cmin > smin && cmin < emin))
            isbetween = 1;
        break;
    }

    return (checkbetween == isbetween);
}

// Converts string with 'HHMM' format into struct hourmin 
int gethourmin(char *strtime, struct hourmin *hmtime) {
    // Arrays are size 3 because HH and MM can be at most 2 digits + '\0'
    char hour[3] = { '0', '\0', '\0' };
    char minute[3] = { '0', '\0', '\0' }; 

    // Takes 'HHMM' string and separates into minutes and hours strings
    switch (strlen(strtime)) {
    case 1: case 2:
        strcpy(hour, strtime);
        break;
    case 3:
        hour[0] = *strtime++;
        hour[1] = '\0';
        strcpy(minute, strtime);
        break;
    case 4:
        hour[0] = *strtime++;
        hour[1] = *strtime++;
        hour[2] = '\0';
        strcpy(minute, strtime);
        break;
    default:
        printf("Error: Bad time arguments. Must be of the form H, HH, HMM, or HHMM.\n");
        exit(1);
    }

    hmtime->hour = (int) strtol(hour, NULL, 10);
    hmtime->minute = (int) strtol(minute, NULL, 10);

    // Checks that mins and hours are legal values. First errno occurs if
    // either strtol() above fail (if str can't be converted to number).
    if (errno) {
        printf("Error: Bad time argument. Must provide a number.\n");
        printf("errno: %s\n", strerror(errno));
        exit(1);
    }
    if (hmtime->hour > 23 || hmtime->hour < 0) {
        printf("Error: Bad time argument. Hour must be between 0-23.\n");
        exit(1);
    } 
    if (hmtime->minute > 59 || hmtime->minute <0) {
        printf("Error: Bad time argument. Minute must be between 0-59.\n");
        exit(1);
    }

    return 0;
}

Remarks / Questions:

• I came across readings suggesting that system() is bad practice for reasons of security, portability, and performance (source). However, it wasn't obvious to me whether reimplementing commands like pgrep and crontab using lower-level library and system calls were worthwhile in this case (or whether that is even the right approach).
• As hinted above, I came across issues when I ran this program without being the root user. In Lubuntu, the user's cron environment doesn't execute the nmcli command for reasons I don't quite understand. I found others with similar issues (source, source, source), but none of the suggestions helped me fix the problem.
• I considered adding functionality that would let me filter wifi access at the application-level (vs system-wide). I came across two possible way of doing it: network extentions (source) and user groups (source), but would be curious if another, simpler way is possible.
• How would I better 'package' this program for more convenient use? Right now I have to compile setcheck.c, rename as setcheck.out, then compile swifi.c, but that seems like a lot of manual work. I'm aware of Makefile / make but I'm not familiar with them and not sure if they would be overkill in a small project like this.

Answer 1

I will offer some suggestions.

Please note I have no experience with Mac OS, but I believe most of it will still apply.

1. Using system is not particularly bad in your case since you use static arguments in those calls excluding the possibility of command injection.
   However, since your first C program is almost completely made up of system calls, you should consider converting it to BASH script as well, since those work a lot better for "command glue" than C binaries.

2. In your C binaries you should not check the OS type at runtime with strcmp.
   This both wastes processing time, and makes your binary larger because it contains extra code that will never be used (unless you turn optimization high enough to remove it automatically).
   Consider using conditional compilation instead.

3. If you are going to work with C on Linux and Mac, you should learn GNU Make, at least its basics.
   Even a single file C program can benefit from a Makefile, as it will save you typing a long compiler command line.
   Also, you can have one Makefile for both of your tools since they are part of the same package.
   It is a very useful automation system any developer should have in their arsenal.

4. If you really intend to distribute your program for Linux, you will probably want to learn the basics of packaging deb or rpm.

5. Consider using better IPC.
   Searching for background process with pgrep every time is both inefficient and cumbersome.
   Consider instead having the process create a "lock" or "flag" file with a fixed name in a fixed path while it is running and delete it on exit.
   You can then easily and quickly test for this file both from BASH and C.
   Also, instead of forcefully killing the processes use signals.
   Finally, if you are going to communicate between C programs, such as a daemon and a client (for sending commands to the daemon), consider using sockets. There is even a way use them from BASH, though it is less convenient and more limited.

I don't have suggestions for fixing your WiFi control issues, and I think that question is more suitable for stackoverflow than this site, but you should consider one thing:
Perhaps refactoring your application in to a system service will solve that problem, and make it more streamlined and efficient.

Answer 2

We can use plain POSIX /bin/sh. The only Bashism I see is

if [[ "$OSTYPE" == "linux-gnu"* ]]; then
elif [[ "$OSTYPE" == "darwin"* ]]; then
fi

This is in any case more naturally expressed as

case "$(uname -s)" in
    Linux)
        ;;
    Darwin)   # fill in actual value here
        ;;
esac

It would be good for the first of these to error-exit the script if there's no match.

Given that the implementations of the functions are generally completely different for each platform, I'd restructure to define them independently for each platform:

case "$(uname -s)" in
    Linux)
        status() { ... }

        isactive() { ... }

    ;;
    Darwin)
        status() { ... }

        isactive() { ... }

    ;;
esac

If we do this, we shouldn't need the $wifion variable - just arrange for status to exit with the appropriate return value:

    status() {
        ip link show enp0s3 | sed 1q | grep -qF 'state UP'
    }

---

In the C code, I see no point creating PLATFORM_NAME macro and the os string, that we then only ever use to compare against fixed values. Why not simply use the existing boolean macros? E.g. instead of

if (!strcmp(os, "apple")) {
        system("/bin/kill $(/usr/bin/pgrep -f checkon.sh) >>/dev/null 2>>/dev/null");
} else if (!strcmp(os, "linux")) {
        system("/usr/bin/pkill checkon >>/dev/null 2>>/dev/null");
}

We could just

#if __APPLE__
            system("/bin/kill $(/usr/bin/pgrep -f checkon.sh) >>/dev/null 2>>/dev/null");
#elif __linux__
            system("/usr/bin/pkill checkon >>/dev/null 2>>/dev/null");
#else
#error This platform not yet supported
#endif

Some minor nits:

• C reserves identifiers beginning with is and str for future library functions. So it's a bad idea to use such names in your code.
• The getarg() function is very duplicated - only a couple of lines need to vary between the platforms.
• Error messages should go to the error stream - fprintf(stderr, ...). I'm pleased to see a non-zero exit in the error case, and recommend using the standard EXIT_FAILURE macro there.
• As mentioned in the other answer, C probably isn't the most appropriate choice for something that mainly invokes other programs. I'd suggest writing it all in shell.