1

All my searching has yielded little in terms of C++ solutions....
I have a class that gets info out of config files, I have a static function that gets the site number out of a different config file. To avoid duplicating the code I create an instance of the class inside of the static function. This compiles without any warnings (with -Wall).

Unfortunately I can't find any information on whether or not it's undefined behavior. Is it ?

#include <iostream>
#include <fstream>
#include "srxDSLog.H"

extern  SrxDsLog *Logger;

class Stanza {
private:
    std::string config_file_path_;

public:
    Stanza(std::string config_file_path) { 
        config_file_path_ = config_file_path;
    }

    std::string GetValue(std::string config_section, std::string config_setting) {
        std::ifstream config_file(config_file_path_.c_str(), std::ios::in);
        std::string current_section, line;
        for ( ; std::getline(config_file, line); ) {
            line = rtrim(line);
            if (line.find(" ") == std::string::npos && line.find(":") != std::string::npos) {
                current_section = line.substr(0, line.find(":"));
                continue;
            }
            if (current_section == config_section) {
                if (line.find(config_setting) != std::string::npos && line.find("=") != std::string::npos) {
                    return line.substr(line.find(config_setting) + config_setting.length() + 3); // we're getting the string starting after the name of the setting + " = " + 1. We're assuming there's exactly 1 space 
                }
            }
        }
        if (current_section.empty()) {
            Logger->WriteLog("Couldn't find section: " + config_section, LOG_ERROR, "Stanza::GetValue");
            return "";
        }
        else {
            Logger->WriteLog("Couldn't find setting: " + config_setting, LOG_ERROR, "Stanza::GetValue");
            return "";
        }
        Logger->WriteLog("Somehow reached the end of function without returning", LOG_ERROR, "Stanza::GetValue");
        return "";
    }

    static std::string rtrim(std::string input) {
        if (input.find_last_not_of(" ") == input.length() - 1) {
            return input;
        }
        else {
            return input.substr(0, input.find_last_not_of(" ") + 1);
        }
    }

    static int GetStoreNumber() {
        Stanza store_config("/u/data/store.cfg");
        std::string store_number =  store_config.GetValue("store", "site");
        return atoi(store_number.c_str());
    }

};
xyious
  • 1,065
  • 4
  • 13
  • 29
  • 2
    Out of curiosity, is there any reason you would think this isn't safe? – templatetypedef Oct 03 '17 at 21:36
  • I worry about future of C++, pretty soon people will start to ask if it is UB to create `int` variable inside a function. – Slava Oct 03 '17 at 21:39
  • My last question ended up with undefined behavior because I did something I've seen many times but it apparently isn't defined before C++11. (calling constructor inside constructor). It led to an extremely weird behavior. I simply wasn't able to find anything on the internet, all I could find was 'can I instantiate a class inside a different class ?' Or answers from perl or java.... – xyious Oct 03 '17 at 23:39
  • @xyious You can indeed call one constructor from another in C++... you just have to be careful about how you do it. – templatetypedef Oct 03 '17 at 23:47
  • check this question: https://stackoverflow.com/questions/46370438/stdofstream-wont-write-to-file-sometimes/46373586#46373586 I had a default constructor taking 4 arguments which is called from 2 different constructors getting 3 arguments. No warnings from GCC, just apparently a completely unrelated function won't write to a file. Now I've changed the constructor and only use 1 constructor and everything works. – xyious Oct 03 '17 at 23:50

3 Answers3

4

This is perfectly legal and totally safe.

It sometimes helps to think of a static function as a free function that just happens to be scoped inside of the class it's declared in. You could declare objects of type Stanza in a free function, so it's fine to do so in a static member function inside of Stanza.

There are very few circumstances where it's risky to define an object of type T inside of a member function of T, and those cases are primarily constructors or destructors where you have to worry about accidental recursion.

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
  • I would wager quite a lot that the issue you’re getting is not due to making an object of the class type here. Can you post a follow-up question with a minimal example needed to reproduce the behavior? – templatetypedef Oct 04 '17 at 21:16
1

It's perfectly acceptable code. I can't point you at a reference that says it's not undefined behavior because it's not a case anybody has thought to point out as being worrisome.

A static function is just like an ordinary friend function not inside the class. If you would be able to create objects if your type in an ordinary unrelated function, you should be able to inside a static function as well.

As an example, I have static functions who's whole purpose in life is to construct members of my class. I use these to make sure that all instances of my class are constructed using new and are referenced via shared_ptr. How do I call ::std::make_shared on a class with only protected or private constructors?

Omnifarious
  • 54,333
  • 19
  • 131
  • 194
  • 1
    I think this is fairly typical in a factory pattern, which further points to it being completely defined, since it's _commonly_ used – Tas Oct 03 '17 at 21:36
1

Unfortunately I can't find any information on whether or not it's undefined behavior. Is it ?

It's perfectly fine in terms of behavior of the function.

What's not clear to me is whether you need a non-static instance of the class in the static member function.

If the contents of the file "/u/data/store.cfg" are not expected to change during execution of the program, you could use a static variable.

static int GetStoreNumber() {
    static Stanza store_config("/u/data/store.cfg");
    std::string store_number =  store_config.GetValue("store", "site");
    return atoi(store_number.c_str());
}

You can refine it further to use:

static int GetStoreNumber() {
    static int number = getStoreNumber("/u/data/store.cfg");
    return number;
}

static int GetStoreNumber(std::strinc const& filename) {
    Stanza store_config(filename);
    std::string store_number =  store_config.GetValue("store", "site");
    return atoi(store_number.c_str());
}

If the contents of the file "/u/data/store.cfg" are expected to change during execution of the program, you can keep the function as you have it.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • `static` variables of any complexity can introduce interesting initialization cycles before the program starts up. And sometimes these can even include cycles. If some other initialization routine calls `GetStoreNumber` for some reason, for example and `Stanza` depends on this other initialization routine in order to work. – Omnifarious Oct 03 '17 at 21:42
  • @Omnifarious, I don't follow your drift. – R Sahu Oct 03 '17 at 21:47
  • 1
    Suppose, for example, that `Stanza`, when constructed called `GetStoreNumber` in it's constructor. Say it was initializing a store name from a template and that templated called for the store's number. Then, constructing a `Stanza` at all would result in infinite recursion. Of course, that's not specific to a `static`. But, suppose instead you needed `Stanza::GetStoreNumber` in the constructor for some other class you had a static instance of, and the constructor for `Stanza` needed this static instead of this other class. You then basically have an initialization cycle at startup. – Omnifarious Oct 03 '17 at 21:56
  • 1
    If you've layered your design carefully so that such a cycle can't happen (statically initializing a `vector` of a primitive type is just fine, for example, because it's not going to call your code) then you're OK. But if you're worried about whether or not it's OK to declare an instance of a class inside of a static member function of that class, I'm not confident in whether your design being layered correctly to avoid the cycle. I don't like non-POD variables being initialized statically for this reason. – Omnifarious Oct 03 '17 at 21:59
  • @Omnifarious, thanks for taking the time to explain your point in detail. Yes, I understand your concern now. Hopefully it's not issue for the OP's case. – R Sahu Oct 03 '17 at 22:06
  • I only need to call this function one time during the execution of the program and I would like the variable to go out of scope after the call. The other (easy) way would have been to just create another instance of stanza and use GetValue to get the site number. But since 3 programs have (3 different) GetStoreNbr functions I wanted to just refactor it to a different file that I can use in all 3. – xyious Oct 03 '17 at 23:42