C++ returning uint16_t array from function causes segmentation fault

Question

uint16_t * getRegister(std::string reg) {
    // unordered_map<string, vector<uint16_t>> current_reg_map;// defined globally.
    // vector current_reg_map has minimum 2 values;

    uint16_t regval[current_reg_map[reg].size()];
    std::copy(current_reg_map[reg].begin(), current_reg_map[reg].end(), regval);
    return regval;
}

int main() {
    std::string regname = "4400";
    uint16_t *tmp = reader.getRegister2(regname, tmp);
    std::cout<<"result "<<tmp[0]<<"\t"<<tmp[1]<<std::endl;
}

Throws segmentation fault at runtime. A lot of posts in stack overflow suggests returning a vector instead. But I explicitly require a uint16_t array as this module is part of a larger project.

You cannot use `tmp` in `main`, it is a dangling pointer. If you cannot change the call sites, you have a problem. You could use `uint16_t * regval = new uint16_t[current_reg_map[reg].size()];`, but then you need a `delete [] tmp;` somewhere in the call site. — mch, Aug 01 '22 at 08:59
The solution is still to use a vector. When later you need that array, just call the vector's `data` method to get the pointer to the underlying array (and the `size` method to get it's size). — john, Aug 01 '22 at 09:06

score 3 · Answer 1 · answered Aug 01 '22 at 08:59

You cannot return an array from a function in C++. Your function returns a pointer. But that pointer points to an array which has been destroyed (after you left the function where it was declared). Therefore you get a segmentation fault.

The C++ solution is to use a vector

std::vector<uint16_t> getRegister(std::string reg) {
    std::vector<uint16_t> regval(current_reg_map[reg].size());
    std::copy(current_reg_map[reg].begin(), 
        current_reg_map[reg].end(), 
        regval.begin());
    return regval;
}

int main(){
   std::string regname = "4400";
   std::vector<uint16_t> tmp = reader.getRegister2(regname);
   std::cout<<"result "<<tmp[0]<<"\t"<<tmp[1]<<std::endl;
 }

score 0 · Answer 2 · answered Aug 01 '22 at 09:00

0

This is one of the methods I found to achieve what I was looking for. Instead of returning an array from the function, I can create an array in the main function and pass it as an argument.

int getRegister(std::string reg, uint16_t *arr){
        // unordered_map<string, vector<uint16_t>> current_reg_map;// defined globally.
        // vector current_reg_map has minimum 2 values;
        std::copy(current_reg_map[reg].begin(), current_reg_map[reg].end(), arr);
        return current_reg_map[reg].size();
    }   

int main(){
   std::string regname = "4400";
   uint16_t tmp[2]; 
   int i = reader.getRegister2(regname, tmp); // returns size of array.
   std::cout<<"result"<<tmp[0]<<"\t"<<tmp[1]<<std::endl;
    }

answered Aug 01 '22 at 09:00

Aritra Bhuiya

1

1

That's the solution used in C. The C++ solution uses a vector which is cleaner (IMHO). One problem with this solution is you have to determine the size of the array before you call the function. In the vector version you discover the size of the vector after you call the function. – john Aug 01 '22 at 09:02
Thanks for your comment! Yes the size declaration is one problem. However, I need the result in uint16_t [ ] as this module replaces another module in my project. The other modules rely on uint16_t [ ] . – Aritra Bhuiya Aug 01 '22 at 09:07
1

As I said above you can use the `vector::data` method to get the pointer that you need. So you should still be using a vector. Your original code returned a pointer, but you can get that pointer from a vector, so really there is no issue with using a vector here. – john Aug 01 '22 at 09:08
@AritraBhuiya you should use `std::vector` ubiquitously. And use the `std::vector::data()` method as suggested above when you have no other choice (due to legacy considerations). – wohlstad Aug 01 '22 at 09:30

score 0 · Answer 3 · answered Aug 01 '22 at 09:57

You must not return the address/reference to a local variable inside a function because it doesn't exist outside the scope of that function. Both of these examples are wrong:

int *foo() {
    int a;
    return &a;
}

int& bar() {
    int b;
    return b;
}

Unless you inhibit all warning messages from the compiler, you should get this warning:

warning: address of local variable 'a' returned [-Wreturn-local-addr]
warning: reference to local variable 'b' returned [-Wreturn-local-addr]

If you need to return the address of a local variable, you need to dynamically allocate the memory for that variable.

int *foo() {
    int *a = new int;
    return a;
}

But remember to free/deallocate the allocated memory. If you need a simpler solution, you can use STL containers or smart pointers.

While you are not wrong, per se, it feels wrong to say "must not" rather than "should not" as this does not create a compilation error, but rather a compiler warning, and invokes undefined behavior rather than outright erroneous behavior. — Chris, Sep 16 '22 at 15:14

C++ returning uint16_t array from function causes segmentation fault

3 Answers3