CODEWITHSUNDEEP
c++c
Justin Meiners
Justin Meiners

Reputation: 11113

Return value of static variable from inline function

static inline JGPlatformInfo* currentPlatform(){
    static JGPlatformInfo* platform = nil;

    if (platform == nil){
        JGPlatformInfo info = supportedPlatform();
        platform = &info;
    }

    return platform;    
}

I am getting inconsistent results with this code (C family) The value of platform changes for no reason and the memory appears to be correct. What could be causing this? Could the inline keyword be affecting this?

Upvotes: 0

Views: 1591

Answers (5)

vedosity
vedosity

Reputation: 720

Variables you defined are always temporary. That means when you leave scope your variable is going to get destroyed and the pointer to it will be invalid (or worse: it will point to a valid location that it shouldn't point to). This is where dynamic allocation comes into hand. Here's how I would achieve the same effect that you are going for:

In your source file:

JGPlatformInfo* gCurrentPlatform = NULL;

If you really want this function inline, put this in your header file:

extern JGPlatformInfo* gCurrentPlatform;

...

// I'm not too familiar with C, but it seems like adding static
// to an inline function would be redundant
inline JGPlatformInfo* currentPlatform()
{
    if (!gCurrentPlatform)
    {
        gCurrentPlatform = malloc(sizeof(JGPlatformInfo));
        *gCurrentPlatform = supportedPlatform();
    }

    return gCurrentPlatform;
}

malloc(x) allocates x bytes of memory and returns a pointer to it.

Upvotes: 3

mr.dot
mr.dot

Reputation: 16

Cause inline function will be expanded at every place you use it. So in fact there will be many static platform in your codes. You should remove "inline", and don't try to return address of temp variables.

Upvotes: -1

caf
caf

Reputation: 239011

With the C99 version of inline functions, an inline function is not allowed to declare variables with static storage duration - doing so invokes undefined behaviour.

You must make the variable an extern and define it in one translation unit.

Upvotes: 3

Nikolai Fetissov
Nikolai Fetissov

Reputation: 84159

I'm guessing you have that in a header file, right? Then each translation unit will have its own copy of the platform variable.

Now for something completely different - the real problem: you are returning an address of an automatic variable - i.e. a pointer into some stack frame - that's broken.

Upvotes: 6

Potatoswatter
Potatoswatter

Reputation: 137780

I'm not sure what's causing this issue, but the usual (simpler, safer, idiomatic) way to do this is:

inline JGPlatformInfo &currentPlatform() {
    // supportedPlatform() will only be called once
    static JGPlatformInfo platform = supportedPlatform();

    return platform; // & platform would work if you must return a pointer
}

I suspect the reason for your immediate problem is that qualifying the function as static causes the compiler to create a separate instance of the variable for every translation unit. It wasn't returning to NULL for every function call, but only on the first call from each .cpp file.

Upvotes: 4

Related Questions