Why is the Function Returning an Empty String

Question

Why is this function always returning an empty string?

I'm aware of scope of variables but I thought the function would return a copy of the std::string res?

string getNumberOfLines(string absFilePath)
{
    int nLines = 0;
    ifstream inFile(absFilePath.c_str());

    if (!inFile)
        return "no lines";

    char c;
    while (inFile.get(c))
    {
        if (c == '\n')
            nLines++;
    }   

    string res;
    sprintf(&res[0], "%d lines.", nLines);
    puts(res.c_str()); // prints "40 lines" or etc.
    return res;
}

puts(getNumberOfLines("f.txt").c_str()); // outputs nothing

func(const string absFilePath) will copy the string, but func(const string& absFilePath) won't. sprintf cannot be used with std::string in this way. &res[0] is out of bounds as the string is empty. A good debug compiler will flag this for you. Use std::cin instead. — Neil Kirk
– Neil Kirk, Commented Mar 22, 2015 at 1:46
@NeilKirk Thanks for your advice. Does func(string absFilePath) also copy the string? I've often seen func(const string& absFilePath) - why have it constant why not just pass by reference normally? — sazr
– sazr, Commented Mar 22, 2015 at 1:52
The & is what prevents the copy of the string. The const is to prevent accidentally modifying the string passed to the function from outside. The caller may want to reuse it for something else and, as it is an input parameter, not expect it to be changed by the function. — Neil Kirk
– Neil Kirk, Commented Mar 22, 2015 at 1:53
@NeilKirk ah I see now, thats what I was worried about - the function altering the string. So const ensures the function wont change their string and pass by reference is for efficiency (no copy made). Is the efficiency achieved at compile time or runtime? — sazr
– sazr, Commented Mar 22, 2015 at 1:56
The string may not be known until runtime so this is a runtime optimization. — Neil Kirk
– Neil Kirk, Commented Mar 22, 2015 at 1:56

Community · Accepted Answer · 2020-06-20 09:12:55Z

2

You're using reference std::basic_string::operator[](size_t pos) to get the first element in a string and then its address using the & operator. However, as per the function's specification, if pos == size then it's undefined behaviour. C++11 standard, draft N3337, [string.access], basic_string element access (emphasis mine):

1 Requires: pos <= size().

2 Returns: *(begin() + pos) if pos < size(). Otherwise, returns a reference to an object of type charT with value charT(), where modifying the object leads to undefined behavior.

Your string res has no elements in it and is thus 0 sized, thus undefined behaviour ensues. After this point, nothing else is guarenteed by the language.

You should be using std::ostringstream for this:

std::ostringstream ss;
ss << nLines << " lines.";
auto res = ss.str();
std::cout << res << '\n';
return res;

edited Jun 20, 2020 at 9:12

CommunityBot

11 silver badge

answered Mar 22, 2015 at 3:12

legends2k

33.2k28 gold badges131 silver badges238 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Mgetz Over a year ago

as is standard (no pun intended) for any c++ question, can you quote the standard for this?

Neil Kirk Over a year ago

@Mgetz Why do we have to quote the standard for every thing? That would be very tedious.

Lightness Races in Orbit Over a year ago

@NeilKirk: We don't. Only those things for which standard-mandates are asserted. Providing evidence is always better than not doing so.

legends2k Over a year ago

@Mgetz There you go! However, I agree with Kirk here, for the obvious ones it's sometimes an overkill.

Lightness Races in Orbit · Accepted Answer · 2015-03-22 03:01:43Z

0

Basically you are trying to write into an empty string.

When you create a std::string, no storage is allocated, so you are passing the address of a zero-length array to sprintf.

edited Mar 22, 2015 at 3:01

Lightness Races in Orbit

387k77 gold badges670 silver badges1.1k bronze badges

answered Mar 22, 2015 at 2:58

kuroi neko

8,6781 gold badge23 silver badges45 bronze badges

5 Comments

Neil Kirk Over a year ago

"you are passing the address of a zero-length array" Hm I'm not sure about this. The state has become undefined before that point.

Neil Kirk Over a year ago

"Don't use C++" isn't a logical answer to "What's the best practice in C++?"

Mgetz Over a year ago

@kuroineko your post violated the stackoverflow posting guidelines because it was a rant and not an answer. LRIO was nice enough to remove the rant and leave only the answerish bits that are actually relevant to the question.

Lightness Races in Orbit Over a year ago

"Censored"? Calm down.

kuroi neko Over a year ago

@NeilKirk I'm surprised our C++ valkirye did not correct this intolerable inaccuracy herself. Indeed, &res[0] is an UB, so the compiler is free to paint your mustache pink when it encounters it.

Collectives™ on Stack Overflow

Why is the Function Returning an Empty String

2 Answers 2

4 Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related