Search code examples
cpointersdereference

The program will crash when I run .\pass_args.exe -a 5 -q (when command line argument are not given in pairs)


I am struggling to identify the bug which cause the program to crash when the code is run with odd number of command line arguments. e.g.

The program can be launched with command line arguments: w : select the type of wave generated a : amplitude of the wave f : frequency of the wave if the value provided is not valid, the function passArgs will return immediatedly.

To reproduce the error:

.\pass_args.exe -a 5 -q

or

.\pass_args.exe -a 5 -w square -e

It works well if I run

.\pass_args.exe -a 5 -w square -e 10

pass_args.c

#include <stdio.h>
#include <unistd.h>
#include "pass_args.h"

int isValidFloat(const char *str)
{
    size_t len = strlen(str);
    if (len == 0)
    {
        return 0; // Empty string is not a valid float
    }

    if (!(str[0] >= '0') || !(str[0] <= '9')) // first char is not number
    {
        if (len == 1)
        { // sign without any digits is not a valid float
            return 0;
        }
        if (str[0] == '-')
        {
            str++; // move past the sign character
            len--;
        }
    }

    int dotFound = 0; // Flag to keep track of the decimal dot
    // Check that all remaining characters are digits (0-9) or a single decimal dot
    for (size_t i = 0; i < len; i++)
    {
        if (str[i] == '.')
        {
            if (dotFound)
            {
                return 0; // More than one decimal dot found, not a valid float
            }
            dotFound = 1; // Set dotFound to true after encountering the first dot
        }
        else if (str[i] < '0' || str[i] > '9')
        {
            return 0; // Non-digit or non-dot character found, not a valid float
        }
    }

    return 1; // String represents a valid float
}

void printArgumentManual()
{
    printf("Welcome\n");
};

int passArgs(int argc, char *argv[])
{
    int valid_float = 0;
    int wave_valid = 0;
    int freq_valid = 0;
    int amp_valid = 0;
    float tmp = 0.0;
    char *value;
    if (argc <= 2) // no command line argument used
    {
        printf("no option given\n");
        return 1;
    }
    printf("argc %d\n", argc);
    char **p_to_args = &argv[1];
    char **p_to_vals = &argv[2];

    while (*p_to_args != NULL)
    {
        if ((*p_to_args[0]) != '-' || (*p_to_args[1]) == '\0')
        {
            printf("Invalid options!\n");
            return 2;
        }
        if (*p_to_vals == NULL || *p_to_vals == '\0')
        {
            printf("unexpected values for arguments!\n");
            return 2;
        }
        switch ((*p_to_args)[1])
        {
        case 'w':
            if (!strcmp(*p_to_vals, "sine"))
            {
                waveforms = 1;
                wave_valid = 1;
                break;
            }

            else if (!strcmp(*p_to_vals, "triangular"))
            {
                waveforms = 2;
                wave_valid = 1;
                break;
            }

            else if (!strcmp(*p_to_vals, "sawtooth"))
            {
                waveforms = 3;
                wave_valid = 1;
                break;
            }
            else if (!strcmp(*p_to_vals, "square"))
            {
                waveforms = 4;
                wave_valid = 1;
                break;
            }
            else
            {
                printf("Invalid wave option.\n");
                return 2;
            }
        case 'a':
            valid_float = isValidFloat(*p_to_vals);
            if (valid_float == 0)
            {
                printf("Invalid float value is given\n");
                return 2;
            }
            tmp = strtod(*p_to_vals, NULL);
            if (tmp >= -5.0 && tmp <= 5.0)
            {
                amp = tmp;
                break;
            }
            else
            {
                printf("Invalid amplitude value!\n");
                return 2;
            }
        case 'f':
            valid_float = isValidFloat(*p_to_vals);
            if (valid_float == 0)
            {
                printf("Invalid float value is given\n");
                return 2;
            }
            tmp = strtod(*p_to_vals, NULL);
            if (tmp >= 0.1 && tmp <= 10.0)
            {
                freq = tmp;
                break;
            }
            else
            {
                printf("Invalid frequency value!\n");
                return 2;
            }
        default:
            printf("Unexpected arguments\n");
            return 2;
        }
        p_to_args += 2;
        p_to_vals += 2;
    }
    return 0;
}

int main(int argc, char *argv[])
{
    int state = 0;

    state = passArgs(argc, argv);
    printf("state %d\n", state);
    printf("freq :%f\n", freq);
    printf("amp: %f\n", amp);
    printf("waveforms: %d\n", waveforms);
    return 0;
}

pass_args.h

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

// three arguments
float freq;    // 0.1 - 10.0 Hz
float amp;     // 0 - 2.5V
int waveforms; // 1 = Sine wave
               // 2 = Triangular wave
               // 3 = Sawtooth wave
               // 4 = Square wave

// This function prints the arguments usage for this program
void printArgumentManual();

// This function checks if the string is a valid float
// Argument:
//         str: pointer to the string
// Return:
//          1: not a valid float
//          0: a valid float
int isValidFloat(const char *str);

// This function pass the argument
// Argument:
//      argc: number of argument
//      argv: pointer to pointer of argument
// Return:
//      0: success
//      1: no command line argument used
//      2: invalid option
//      3: choose default setting while providing values
int passArgs(int argc, char *argv[]);

I have tried to use debugger to run line by line. When the pointer to pointer is incremented, segmentation fault will occur.


Solution

  • *p_to_args[1] does not do what you think it does.

    You want it to be this: (*p_to_args)[1]. First dereference the pointer, then look at its 2nd element.

    But it is really this: *(p_to_args[1]). First look at its second element, then dereference it.

    With args like -a 5 -w square -e if p_to_args[0] is -e then p_to_args[1] will be a null pointer. *p_to_args[1] tries to dereference a null pointer. Program crashes.


    Instead, make sure to put parens around the dereference.

    (*p_to_args)[1] == '\0'
    

    Better: use normal array syntax.

    if (p_to_args[0][0] != '-' || p_to_args[0][1] == '\0')        {
    

    Best: Error early if there are an odd number of arguments (argc is even).

        if (argc <= 2) // no command line argument used
        {
            fprintf(stderr, "no option given\n");
            return 1;
        }
        if (argc % 2 == 0 ) {
            fprintf(stderr, "options are not in pairs\n");
            return 1;
        }
    

    Then you don't have to check if p_to_vals is null.

            if (p_to_args[0][0] != '-' || p_to_args[0][1] == '\0')        {
                fprintf(stderr, "expected a switch like -a");
                return 2;
            }
            if (p_to_vals[0][0] == '\0')
            {
                fprintf(stderr, "the value is empty\n");
                return 2;
            }
    

    Demonstration.