1

I have a function that uses a static variable to count how many times it got called:

void func ()
{
    static int counter=0;
    counter++;

    /* some code */
}

The code in the function executes differently depending on the counter.

As it happens while expanding the program to have more features etc. the original implementation is insufficient and there are cases where counter needs to be modified to get the correct behavior.

This however turns out to be quite a challenge as the function in question is already in use all over the program, which means that changing things like the function parameters would break a lot of prexisitng code.

After thinking about this problem I came up with the following two solutions:

Solution 1:

int& func ()
{
    static int counter=0;
    counter++;

    /* some code */
    return counter//return an lvalue reference
}

int main() 
{
    /* some code */
    func()=2;//modify anonymous variable of type lvlaue reference, referencing counter
    /* some code */
    return 0;
}

Solution 2:

int* func ()
{
    static int counter=0;
    counter++;

    /* some code */
    return &counter//return a pointer
}

int main() 
{
    /* some code */
    *func()=2;//modify anonymous variable of type pointer, pointing to counter
    /* some code */
    return 0;
}

As far as I can tell it works, but to me it looks rather suspicious, especially since I haven't ever seen such practices.

I understand the code as follows in the first solution it returns counter by reference, so after func evaluates to an anonymous object in the expression in main(), which happens to be an lvalue reference to counter. That effectively lets main() modify a variable that's out of scope.

The second solution works analogously, but using a pointer instead of an lvalue reference. And since in both cases counter, as a static variable, has a lifetime that ends with the program the variable/memory address returned stays valid, even though it is accessed out of scope.

But is my interpretation correct? Is it undefined behavior after all and I just got lucky that it worked?

And if it is correct would it be considered bad code design given the circumstances?

6
  • 1
    No undefined behaviour here. The variable keeps existing, even after the function returns.
    – user253751
    Aug 17, 2022 at 18:20
  • 3
    you can also just make the variable global, instead of hacking in a way to access a static local variable. Static local variables are just global variables that are invisible outside the function
    – user253751
    Aug 17, 2022 at 18:38
  • I also thought about that, but that seems even more error prone as basically anything and everything could modify a global variable.
    – Aiko
    Aug 17, 2022 at 18:42
  • 3
    anything and everything could modify this counter!
    – user253751
    Aug 17, 2022 at 18:50
  • 1
    I think it's time to convert that code into a class instead of a function, with explicit state and proper function(s) to manipulate that state. Yes, that means you'll have to rewrite all the code that uses that function. Aug 18, 2022 at 9:10

2 Answers 2

3

Congratulations, you've figured out why hidden global state is bad.

Commiserations, you are going to have to change a whole bunch of code to fix this.

class Counter {
    int counter;
public:
    Counter(int counter = 0) : counter(counter) {}
    void func() { ++counter; /* some code */ }
}

int main() 
{
    Counter c;
    /* some code */
    c = Counter(2);
    /* some code */
    return 0;
}

Everywhere that was just calling func now needs a reference to a Counter.

I'm going to guess that one of the reasons you "need to modify" the counter is that you want to return to some previous state after having called func a bunch of times. Luckily you can easily achieve that with the Counter class, by copying it and operating on the copy.

I.e. instead of

void doStuffAndRevert() {
    int existing = func() - 1; // include the initial ++
    func();
    func() = existing; // reset to start
}

you can, by using pass-by-value to ensure you have a local copy:

void doStuffAndRevert(Counter c) {
    c.func();
    c.func();
    c.func();
}
4
  • I noticed that in the first example you have the line c = Counter(2); . I have never encounter code where a class is initialized without a name. Upon further testing I also found out that apparently int (5) is also legal code. What is this kind of code called and could you please explain it?
    – Aiko
    Aug 18, 2022 at 13:44
  • 1
    @Aiko it's a direct initialisation
    – Caleth
    Aug 18, 2022 at 14:12
  • I thought it was a function-style cast. Either way, it works.
    – user253751
    Aug 18, 2022 at 15:11
  • @user253751 it is both. From function-style cast we find it is a static_cast, and from that we consider overload resolution for direct initialisation, and find Counter::Counter(int)
    – Caleth
    Aug 18, 2022 at 15:41
0

If you think you want to be able to modify counter, then you create a function setCounter(value). Not caring at the moment how you do it. Having a function returning the address of the variable is an unacceptable hack.

And then it looks like the best implementation is something like static int counter_ = 0; outside any function. And both func() and setcounter() set that variable. It can only be modified by code inside the file, so that is as safe as possible. The trailing underscore to state clearly that you usually shouldn’t touch it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.