atof coredump with getopt

Question

I'm writing a C++ application which converts fahrenheit to celsius and kelvin, and kelvin to celsius and fahrenheit, etc. Since it's stupid to write an interative application here, I decided to familiarize myself with the getopt function in unistd.h.

Format: F2C -k 273.15

Output:

FAHR CELSIUS KELVIN

32 0 273.15

Here is my code:

#include <iostream>
#include <stdlib.h>
#include <unistd.h>

#define VERSION 0.1
#define HELP help(argv[0])

#define OPTS "vk:f:c:h"
float ver = (float)VERSION;

void help(char *s);
namespace Fahrenheit
{
    float FK(float F) {
        return ((5.0/9.0) * (F - 32.0) + 273.15);
    }

    float FC(float F) {
        return ((5.0/9.0) * (F - 32.0));
    }

    void printfahr(float F) {
        std::cout << "FAHR\t\tCELSIUS\t\tKELVIN" << std::endl;
        std::cout << F << "\t\t" << FC(F) << "\t\t" << FK(F) << std::endl;
    }      
}

namespace Celsius
{
    float CF(float C) {
        return ((C*(9/5)) + 32);
    }
    float CK(float C) {
        return (C+273.15);
    }
    void printc(float C) {
        std::cout << "FAHR\t\tCELSIUS\t\tKELVIN" << std::endl;
        std::cout << CF(C) << "\t\t" << C << "\t\t" << CK(C) << std::endl;
    }
}

namespace Kelvin
{
    float KF(float K) {
        return (((9.0/5.0) * (K-273.15)) + 32);
    }    
    float KC(float K) {
        return (K-273.15);
    }    
    void printk(float K) {
        std::cout << "FAHR\t\tCELSIUS\t\tKELVIN" << std::endl;
        std::cout << KF(K) << "\t\t" << KC(K) << "\t\t" << K << std::endl;
    }
}
int main(int argc, char *argv[])
{
    char arg = '\0';
    if(argc < 2 && argc == 1 && argc > 0) {
        help(argv[0]);
        exit(1);
    }
    /*** Use function getopt() defined in unistd.h to accept 5 arguments: -v, -h, -k, -f, and -c ***/
    while((arg=getopt(argc, argv, OPTS))!=-1)
    {
        float floatarg = atof(optarg);                                                      
        switch(arg)
        {

            case 'v':
                std::cout << "The current version is:" << ver << std::endl;
            break;

            case 'h':
                HELP;
            break;

            case 'k':
                Kelvin::printk(floatarg);
            break;

            case 'f':
                Fahrenheit::printfahr(floatarg);
            break;

            case 'c':
                Celsius::printc(floatarg);
            break;

            default:
                HELP;
            break;
        }
    }
    return 0;
}

void help(char *s) {
    std::cout << "Usage:\t"<< s << " [-option] [argument]" << std::endl;
    std::cout << "option:\t" << "-c [temperature]: convert a Celsius temperature to Fahrenheit and Kelvin" <<  std::endl;
    std::cout << "\t" << "-f [temperature]: convert a Fahrenheit temperature to Celsius and Kelvin" << std::endl;
    std::cout << "\t" << "-h: show help information" << std::endl;
    std::cout << "\t" << "-k [temperature]: convert a Kelvin temperature to Fahrenheit and Celsius" << std::endl;
    std::cout << "\t" << "-v: show version information" << std::endl;
}

My problem is that whenever I use an option that accepts no arguments (like -v) I get a core dump.

dbx has shown me that the SIGSEV occurs at line 70 (float floatarg = atof(optarg);).

When I run the program like this:

./F2C -k 273.15

The math is done correctly and I get a clear printout. It's only when I use -v or -h that my program SIGSEV's.

Extra information:

This program was compiled with the Sun studio compiler suite, version 5.12.

I'm completely baffled as to why my program SIGSEV's. It is inconsistent and makes no sense. I would appreciate any help available.

Do you expect a `float` argument given for every option? – user0042 Aug 10 '17 at 09:13 — user0042, Aug 10 '17 at 09:13
@user0042 No, but if that float arg is never... never mind. – Distant Graphics Aug 10 '17 at 09:14 — Distant Graphics, Aug 10 '17 at 09:14

score 0 · Accepted Answer · answered Aug 10 '17 at 09:15

0

Should have done some optarg checking. After all, you can't convert null to a float.

new main():

#define FLOATARG atof(optarg)

int main(int argc, char *argv[])
{
    char arg = '\0';
    if(argc < 2 && argc == 1 && argc > 0) {
        help(argv[0]);
        exit(1);
    }
    /*** Use function getopt() defined in unistd.h to accept 5 arguments: -v, -h, -k, -f, and -c ***/
    while((arg=getopt(argc, argv, OPTS))!=-1)
    {                                                      
        switch(arg)
        {

            case 'v':
                std::cout << "The current version is:  << ver << std::endl;
            break;

            case 'h':
                HELP;
            break;

            case 'k':
                Kelvin::printk(FLOATARG);
            break;

            case 'f':
                Fahrenheit::printfahr(FLOATARG);
            break;

            case 'c':
                Celsius::printc(FLOATARG);
            break;

            default:
                HELP;
            break;
        }
    }
    return 0;
}

answered Aug 10 '17 at 09:15

Distant Graphics

93
9

1

Using macros is discouraged in c++. – user0042 Aug 10 '17 at 09:28
@user0042 Why? It's much better to define a symbolic constant and reuse it than use the actual code behind it, since that code might be hard to understand, vague, or both for the reader. FLOATARG is much easier to understand than `atof()`. – Distant Graphics Aug 10 '17 at 09:44
_"FLOATARG is much easier to understand than `atof()`."_ That's highly arguable. You shouldn't obfuscate what code does. And symbolic constants should be typed like `constexpr float PI = 3.1414;`. – user0042 Aug 10 '17 at 09:47
@user0042 I would beg to differ, and so would Brian W. Kernighan and Dennis M. Ritchie. According to the second edition of the C programming language, which is ANSI C, symolic constants should be `#defined`. – Distant Graphics Aug 10 '17 at 10:04
1

1. This book is quite old. 2. That doesn't have anything to do with C++. C++ and C are different programming languages and different rules and best practices apply. – user0042 Aug 10 '17 at 10:06
@user0042 It doesn't matter if it's old or not. It still defines the best programming practices as envisioned by them. Also, C++ is still mostly C with OO principles, so the valid practices in C apply to C++ as well. – Distant Graphics Aug 10 '17 at 10:16
@DistantGraphics No, C++ is not mostly C, it's based on C instead. What you said is same as you would say that Java or C# is mostly C++ which is invalid. – mrogal.ski Aug 10 '17 at 10:33
@m.rogalski You could say that C++ is based on C, and that is true, but practically 99% of C syntax was inherited by C++. That's what I meant, and you can't deny that. – Distant Graphics Aug 11 '17 at 20:58

user0042 · Answer 2 · 2017-08-10T09:26:10.457

The shortest fix is:

        float floatarg = optarg ? atof(optarg) : 0.0;

You can also rewrite your code like

    float floatarg = 0.0;
    switch(arg)
    {

        case 'v':
            std::cout << "The current version is:" << ver << std::endl;
        break;

        case 'h':
            HELP;
        break;

        case 'k':
            floatarg = atof(optarg);
            Kelvin::printk(floatarg);
        break;

        case 'f':
            floatarg = atof(optarg);
            Fahrenheit::printfahr(floatarg);
        break;
...

or

    float floatarg = 0.0;
    if(optarg) {
        floatarg = atof(optarg);
    }
    switch(arg)
    {

        case 'v':
            std::cout << "The current version is:" << ver << std::endl;
        break;

        case 'h':
            HELP;
        break;

        case 'k':
            Kelvin::printk(floatarg);
        break;

        case 'f':
            Fahrenheit::printfahr(floatarg);
        break;
...

atof coredump with getopt

2 Answers2