Return dynamic array in C++

Question

I need to return a unsigned int* from a function. The code below will compile but will crash at run time on a Windows 64 bit machine. I know I am making a silly mistake somewhere and can someone point it out for me. :p. I also have declared the function in my header, so I know its not that error.

Please note I have censored the variable names and numbers because the problem in which this function resides is not for public release yet.

Function:

 unsigned int* convertTime(unsigned int inputInteger, unsigned short inputFrac) {
    unsigned int* output = new unsigned int[2];
    double messageTimeFraction = double(inputFrac) * 20e-6;

    output[1] = unsigned int(inputInteger + 2209032000);
    output[2] = unsigned int(messageTimeFraction * 2e32);

    return output; // Seconds
}

Implementation:

unsigned int* timeStamp;
timeStamp = convertTime(inputInteger,inputFrac);

Answer 1

The indexes in an array of 2 elements are array[0] and array[1], so change it to:

output[0] = unsigned int(inputInteger + 2209032000);
output[1] = unsigned int(inputFrac * 2e32);

Answer 2

I know I am making a silly mistake somewhere and can someone point it out for me

Sure, and it has nothing to do with the Q's subject:

output[2] = unsigned int(inputFrac * 2e32);

The correct entries in output are [0] and [1] -- you're indexing out of bounds. "Undefined behavior" results (such as, the crash you observe).

Answer 3

Well, for starters you have output[1] and output[2]. Arrays are zero-indexed in c/c++, so these should be: output[0] and output[1].

But, since you're asking about c++... I urge you to use std::vector or std::pair.

(Of course, for readability's sake, you might just want to use a trivial struct with useful field names)

Answer 4

Arrays in C++ are zero based, so the elements of your array of size two are output[0] and output[1]

You also might want to return something that better represents the data you are returning, such as a struct with seconds and fractional_seconds members rather than creating a new array.

It's also somewhat strange what you are doing -- 2209032000 is the number of seconds in 70 years, and the result of multiplying a short by 2e32 will overflow the size of unsigned int.

Answer 5

use output[0] and output[1], C / C++ arrays are 0 - based

Answer 6

The more usual way to write functions like this in a C style is to pass in the reference to the variable that will be set.

As a convenience you return the output buffer so that the function can easily be used in an expression.

unsigned int* convertTime(unsigned int* output, unsigned int inputInteger, unsigned short inputFrac) {
  double messageTimeFraction = double(inputFrac) * 20e-6;

  output[0] = unsigned int(inputInteger + 2209032000);
  output[1] = unsigned int(inputFrac * 2e32);

  return output; // Seconds
}

// later
unsigned int seconds[2];
unsigned int* pseconds;
pseconds = convertTime(seconds,a,b);

Answer 7

I created a time structure for the various formats and wrote converter functions to handle the conversions. By using structures, I do not have to worry about memory leaks and improved readability. Furthermore the code is now more scalable than using dynamic arrays, as I can add more fields and create new time formats.

struct time{
    unsigned int timeInteger;
    unsigned int timeFraction;
}time_X, time_Y;

My silly mistake was the typo of zero-based indexing but the bigger mistake was using dynamic array.