CODEWITHSUNDEEP
c++arduino
bdx
bdx

Reputation: 3516

Printing output from string building function not returning expected result

I'm writing a function to output a basic hours & minutes string in 24 hour format from two global int's containing hours and minutes.

I've defined these during initialization:

int g_alarmHours = 7;
int g_alarmMinutes = 0;

The function to return the string is:

char* getAlarmTime() {
  int hours = g_alarmHours;
  int minutes = g_alarmMinutes;
  char t[6];
  t[0] = (hours/10) + '0';
  t[1] = (hours%10) + '0';
  t[2] = ':';
  t[3] = (minutes/10) + '0';
  t[4] = (minutes%10) + '0';
  t[5] = 0;
  return t;
}

The global variables are stubs to be replaced when serial comms to another device are added where those values will be retrieved from.

Calling the function generates the following hex values at the character pointer:

0x20 0x4b 0x00

When I replace the top two lines of the getAlarmTime() function with the following

int hours = 7;
int minutes = 0;

The output is then what I expect, of:

07:00\0

Why is using those global variables causing the output of getAlarmTime() to go so wonky?

Upvotes: 0

Views: 88

Answers (5)

Rohit Vipin Mathews
Rohit Vipin Mathews

Reputation: 11787

You are returning a Local variable as pointer.

return t;

The Ideone compiler returned the following error while compiling:

prog.cpp: In function ‘char* getAlarmTime()’: prog.cpp:8:8: warning: address of local variable ‘t’ returned [-Wreturn-local-addr] char t[6];

But i don't understand how it works when you replace the 1st 2 lines with

int hours = 7;
int minutes = 0;

Use string or pass by deference to solve your issue. Or even a Global variable can solve your issue.

Upvotes: 0

Mike Seymour
Mike Seymour

Reputation: 254471

You're returning a pointer to a local array. It is destroyed before the caller can access it, giving undefined behaviour; in practice it may or may not be overwritten with someone else's data.

The usual solution would be to return a dynamic array (such as std::string); but since you say you have extreme memory restrictions that would be a bad idea here.

I would modify the function so that the caller supplies the buffer:

void getAlarmTime(char t[6]) {
  int hours = g_alarmHours;
  int minutes = g_alarmMinutes;
  t[0] = (hours/10) + '0';
  t[1] = (hours%10) + '0';
  t[2] = ':';
  t[3] = (minutes/10) + '0';
  t[4] = (minutes%10) + '0';
  t[5] = 0;
}

Beware that the caller is now responsible for making sure the buffer is large enough. Even though I declared the parameter as char[6], that serves only as documentation; to the compiler it's the same as char*.

Another possibility is to make the local buffer static; but beware that the function will no longer be reentrant or thread-safe, which could lead to weird bugs.

Why is using those global variables causing the output of getAlarmTime() to go so wonky?

My guess would be that, when you initialise the local variables with constants, the compiler eliminates them and uses the constants instead. This moves the array to somewhere else in the stack, where it happens not to be overwritten before you examine it. But this is all in the realms of undefined behaviour, so the exact details aren't of any practical interest.

Upvotes: 0

utnapistim
utnapistim

Reputation: 27365

Why is using those global variables causing the output of getAlarmTime() to go so wonky?

You are actually looking at undefined behavior here, because you are returning the address of a local (stack) variable.

The following sequence takes place:

  • You call getAlarmTime.

  • compiler allocates stack space for it's variables (hours, minutes and t).

  • Then t is filled

  • you return t's address.

  • control exits function and the address you returned points to unused stack space.

Subsequent stack data (variables declared afterwards or other function calls) will overwrite this space.

Solution: Consider returning a std::string instead of a char*.

Upvotes: 1

mathematician1975
mathematician1975

Reputation: 21351

You are returning a pointer to an array that is local to your function only. Thus when your function exits the array that was created in your function no longer exists and any attempt to access that memory will result in undefined behaviour.

Upvotes: 2

Jesse Good
Jesse Good

Reputation: 52365

You are returning a pointer to a local variable on the stack. The memory the pointer is pointing at is no longer valid and accessing that memory invokes undefined behavior. The reason why you are seeing such strange behavior is because anything can happen when you invoke undefined behavior.

The solution to your problem would be to code in C++ and use std::string.

std::string t;
t.push_back((hours/10) + '0');
...

return t;

Upvotes: 4

Related Questions